* [PATCH] Add vncviewer xm compatibility options the 'xl create' command
@ 2012-03-20 15:50 Goncalo Gomes
2012-03-20 16:05 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Goncalo Gomes @ 2012-03-20 15:50 UTC (permalink / raw)
To: xen-devel@lists.xen.org
[-- Attachment #1: Type: text/plain, Size: 3878 bytes --]
I've attached the preliminary patch to add vncviewer options to the
'xl create'. It applies cleanly against c/s 4e1d091d10d8. All feedback
is welcome.
Goncalo
# HG changeset patch
# User Goncalo Gomes <goncalo.gomes@eu.citrix.com>
# Date 1332257809 0
# Node ID 46f8afe643dee8de2c592c65204567fbad657616
# Parent 4e1d091d10d83130842170cd61f1194e5459f2aa
Add vncviewer xm compatibility options the 'xl create' command
Signed-off-by: Goncalo Gomes <goncalo.gomes@eu.citrix.com>
diff -r 4e1d091d10d8 -r 46f8afe643de tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Fri Mar 16 15:24:25 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c Tue Mar 20 15:36:49 2012 +0000
@@ -1347,6 +1347,8 @@
int dryrun;
int quiet;
int console_autoconnect;
+ int vncviewer;
+ int vncviewer_autopass;
const char *config_file;
const char *extra_config; /* extra config string */
const char *restore_file;
@@ -3306,11 +3308,12 @@
int main_create(int argc, char **argv)
{
const char *filename = NULL;
+ char *dom = NULL;
char *p;
char extra_config[1024];
struct domain_create dom_info;
int paused = 0, debug = 0, daemonize = 1, console_autoconnect =
0,
- quiet = 0, monitor = 1;
+ quiet = 0, monitor = 1, vnc = 1, vncautopass = 1;
int opt, rc;
int option_index = 0;
static struct option long_options[] = {
@@ -3318,6 +3321,9 @@
{"quiet", 0, 0, 'q'},
{"help", 0, 0, 'h'},
{"defconfig", 1, 0, 'f'},
+ {"vncviewer", 0, 0, 'V'},
+ {"vncviewer-autopass", 0, 0, 'A'},
+
{0, 0, 0, 0}
};
@@ -3327,7 +3333,7 @@
}
while (1) {
- opt = getopt_long(argc, argv, "Fhnqf:pcde", long_options,
&option_index);
+ opt = getopt_long(argc, argv, "Fhnqf:pcdeVA", long_options,
&option_index);
if (opt == -1)
break;
@@ -3360,6 +3366,12 @@
case 'q':
quiet = 1;
break;
break;
+ case 'v':
+ vnc = 1;
+ break;
+ case 'a':
+ vnc = vncautopass = 1;
+ break;
default:
fprintf(stderr, "option `%c' not supported.\n", optopt);
break;
@@ -3391,12 +3403,40 @@
dom_info.migrate_fd = -1;
dom_info.console_autoconnect = console_autoconnect;
dom_info.incr_generationid = 0;
+ dom_info.vncviewer = vnc;
+ dom_info.vncviewer_autopass = vncautopass;
+
rc = create_domain(&dom_info);
if (rc < 0)
return -rc;
- return 0;
+ if (vnc && dryrun) {
+ printf("vncviewer not being executed for a dryrun\n");
+ goto out;
+ }
+
+ if (vnc) {
+ dom = libxl_domid_to_name(&ctx, rc);
+ if (dom) {
+ rc = vncviewer(dom, vncautopass);
+ if (!rc) {
+ goto cleanup;
+ } else {
+ rc = 1;
+ goto out;
+ }
+ }
+ }
+
+cleanup:
+ if (dom) {
+ free(dom);
+ dom = NULL;
+ }
+
+out:
+ return rc;
}
static void button_press(const char *p, const char *b)
diff -r 4e1d091d10d8 -r 46f8afe643de tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Fri Mar 16 15:24:25 2012 +0000
+++ b/tools/libxl/xl_cmdtable.c Tue Mar 20 15:36:49 2012 +0000
@@ -31,6 +31,11 @@
" (deprecated in favour of global -N
option).\n"
"-d Enable debug messages.\n"
"-e Do not wait in the background for the
death of the domain."
+ "-V, --vncviewer Connect to the VNC display after the
domain is created.\n"
+ "-A, --vncviewer-autopass\n"
+ " Pass VNC password to viewer via stdin
and -autopass.\n"
+ "--autopass (xm compatibility)\n"
+
},
{ "list",
&main_list, 0,
[-- Attachment #2: xl-vncviewer.patch --]
[-- Type: text/x-diff, Size: 3683 bytes --]
# HG changeset patch
# User Goncalo Gomes <goncalo.gomes@eu.citrix.com>
# Date 1332257809 0
# Node ID 46f8afe643dee8de2c592c65204567fbad657616
# Parent 4e1d091d10d83130842170cd61f1194e5459f2aa
Add vncviewer xm compatibility options the 'xl create' command
Signed-off-by: Goncalo Gomes <goncalo.gomes@eu.citrix.com>
diff -r 4e1d091d10d8 -r 46f8afe643de tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Fri Mar 16 15:24:25 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c Tue Mar 20 15:36:49 2012 +0000
@@ -1347,6 +1347,8 @@
int dryrun;
int quiet;
int console_autoconnect;
+ int vncviewer;
+ int vncviewer_autopass;
const char *config_file;
const char *extra_config; /* extra config string */
const char *restore_file;
@@ -3306,11 +3308,12 @@
int main_create(int argc, char **argv)
{
const char *filename = NULL;
+ char *dom = NULL;
char *p;
char extra_config[1024];
struct domain_create dom_info;
int paused = 0, debug = 0, daemonize = 1, console_autoconnect = 0,
- quiet = 0, monitor = 1;
+ quiet = 0, monitor = 1, vnc = 1, vncautopass = 1;
int opt, rc;
int option_index = 0;
static struct option long_options[] = {
@@ -3318,6 +3321,9 @@
{"quiet", 0, 0, 'q'},
{"help", 0, 0, 'h'},
{"defconfig", 1, 0, 'f'},
+ {"vncviewer", 0, 0, 'V'},
+ {"vncviewer-autopass", 0, 0, 'A'},
+
{0, 0, 0, 0}
};
@@ -3327,7 +3333,7 @@
}
while (1) {
- opt = getopt_long(argc, argv, "Fhnqf:pcde", long_options, &option_index);
+ opt = getopt_long(argc, argv, "Fhnqf:pcdeVA", long_options, &option_index);
if (opt == -1)
break;
@@ -3360,6 +3366,12 @@
case 'q':
quiet = 1;
break;
+ case 'v':
+ vnc = 1;
+ break;
+ case 'a':
+ vnc = vncautopass = 1;
+ break;
default:
fprintf(stderr, "option `%c' not supported.\n", optopt);
break;
@@ -3391,12 +3403,40 @@
dom_info.migrate_fd = -1;
dom_info.console_autoconnect = console_autoconnect;
dom_info.incr_generationid = 0;
+ dom_info.vncviewer = vnc;
+ dom_info.vncviewer_autopass = vncautopass;
+
rc = create_domain(&dom_info);
if (rc < 0)
return -rc;
- return 0;
+ if (vnc && dryrun) {
+ printf("vncviewer not being executed for a dryrun\n");
+ goto out;
+ }
+
+ if (vnc) {
+ dom = libxl_domid_to_name(&ctx, rc);
+ if (dom) {
+ rc = vncviewer(dom, vncautopass);
+ if (!rc) {
+ goto cleanup;
+ } else {
+ rc = 1;
+ goto out;
+ }
+ }
+ }
+
+cleanup:
+ if (dom) {
+ free(dom);
+ dom = NULL;
+ }
+
+out:
+ return rc;
}
static void button_press(const char *p, const char *b)
diff -r 4e1d091d10d8 -r 46f8afe643de tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Fri Mar 16 15:24:25 2012 +0000
+++ b/tools/libxl/xl_cmdtable.c Tue Mar 20 15:36:49 2012 +0000
@@ -31,6 +31,11 @@
" (deprecated in favour of global -N option).\n"
"-d Enable debug messages.\n"
"-e Do not wait in the background for the death of the domain."
+ "-V, --vncviewer Connect to the VNC display after the domain is created.\n"
+ "-A, --vncviewer-autopass\n"
+ " Pass VNC password to viewer via stdin and -autopass.\n"
+ "--autopass (xm compatibility)\n"
+
},
{ "list",
&main_list, 0,
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Add vncviewer xm compatibility options the 'xl create' command
2012-03-20 15:50 [PATCH] Add vncviewer xm compatibility options the 'xl create' command Goncalo Gomes
@ 2012-03-20 16:05 ` Ian Campbell
2012-03-20 16:43 ` Goncalo Gomes
0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2012-03-20 16:05 UTC (permalink / raw)
To: Goncalo Gomes; +Cc: xen-devel@lists.xen.org
On Tue, 2012-03-20 at 15:50 +0000, Goncalo Gomes wrote:
> I've attached the preliminary patch to add vncviewer options to the
> 'xl create'. It applies cleanly against c/s 4e1d091d10d8. All feedback
> is welcome.
>
> Goncalo
>
> # HG changeset patch
> # User Goncalo Gomes <goncalo.gomes@eu.citrix.com>
> # Date 1332257809 0
> # Node ID 46f8afe643dee8de2c592c65204567fbad657616
> # Parent 4e1d091d10d83130842170cd61f1194e5459f2aa
> Add vncviewer xm compatibility options the 'xl create' command
Thanks. Are these options actually compatible with "xm create"? Are you
intending to also add the vncviewer option to the config file? (I think
that was a feature of xm mentioned by the original requester of this
functionality).
Please can you also patch docs/man/xl.pod.1 and/or xl.cfg.pod.5 as
appropriate.
Unfortunately your patch appears to have been whitespace wrapped so it
won't apply. http://wiki.xen.org/wiki/SubmittingXenPatches contains some
tips for using "hg email" which can avoid this, otherwise you could try
Documentation/email-clients.txt from the Linux source for help (I'd give
you a direct link, but git.kernel.org seems to be down), last resort you
can attach the file (but also include a copy inline to aid review)
Also please can you add
[diff]
showfunc = True
to ~/.hgrc (it makes function names show up after the @@ lines which
aids review)
Some review of the code follows.
Thanks,
Ian.
>
> Signed-off-by: Goncalo Gomes <goncalo.gomes@eu.citrix.com>
>
> diff -r 4e1d091d10d8 -r 46f8afe643de tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c Fri Mar 16 15:24:25 2012 +0000
> +++ b/tools/libxl/xl_cmdimpl.c Tue Mar 20 15:36:49 2012 +0000
> @@ -1347,6 +1347,8 @@
> int dryrun;
> int quiet;
> int console_autoconnect;
> + int vncviewer;
> + int vncviewer_autopass;
> const char *config_file;
> const char *extra_config; /* extra config string */
> const char *restore_file;
> @@ -3306,11 +3308,12 @@
> int main_create(int argc, char **argv)
> {
> const char *filename = NULL;
> + char *dom = NULL;
> char *p;
> char extra_config[1024];
> struct domain_create dom_info;
> int paused = 0, debug = 0, daemonize = 1, console_autoconnect =
> 0,
> - quiet = 0, monitor = 1;
> + quiet = 0, monitor = 1, vnc = 1, vncautopass = 1;
> int opt, rc;
> int option_index = 0;
> static struct option long_options[] = {
> @@ -3318,6 +3321,9 @@
> {"quiet", 0, 0, 'q'},
> {"help", 0, 0, 'h'},
> {"defconfig", 1, 0, 'f'},
> + {"vncviewer", 0, 0, 'V'},
> + {"vncviewer-autopass", 0, 0, 'A'},
Here you add 'V' and 'A' but later (in the switch) on you look for 'v'
and 'a'.
It would be useful to have these options for restore too?
> +
Mind the extra whitespace here.
> {0, 0, 0, 0}
> };
>
> @@ -3327,7 +3333,7 @@
> }
>
> while (1) {
> - opt = getopt_long(argc, argv, "Fhnqf:pcde", long_options,
> &option_index);
> + opt = getopt_long(argc, argv, "Fhnqf:pcdeVA", long_options,
> &option_index);
> if (opt == -1)
> break;
>
> @@ -3360,6 +3366,12 @@
> case 'q':
> quiet = 1;
> break;
> break;
> + case 'v':
> + vnc = 1;
> + break;
> + case 'a':
> + vnc = vncautopass = 1;
> + break;
> default:
> fprintf(stderr, "option `%c' not supported.\n", optopt);
> break;
> @@ -3391,12 +3403,40 @@
> dom_info.migrate_fd = -1;
> dom_info.console_autoconnect = console_autoconnect;
> dom_info.incr_generationid = 0;
> + dom_info.vncviewer = vnc;
> + dom_info.vncviewer_autopass = vncautopass;
> +
>
> rc = create_domain(&dom_info);
> if (rc < 0)
> return -rc;
>
> - return 0;
> + if (vnc && dryrun) {
> + printf("vncviewer not being executed for a dryrun\n");
> + goto out;
> + }
> +
> + if (vnc) {
When you seeded dom_info with the necessary fields I expected that
create_domain would do this based on those fields, I think that is
better than doing it here.
> + dom = libxl_domid_to_name(&ctx, rc);
> + if (dom) {
> + rc = vncviewer(dom, vncautopass);
> + if (!rc) {
> + goto cleanup;
> + } else {
> + rc = 1;
> + goto out;
Mind your indentation and/or hard tabs please.
> + }
> + }
> + }
> +
> +cleanup:
> + if (dom) {
> + free(dom);
> + dom = NULL;
> + }
> +
> +out:
> + return rc;
> }
>
> static void button_press(const char *p, const char *b)
> diff -r 4e1d091d10d8 -r 46f8afe643de tools/libxl/xl_cmdtable.c
> --- a/tools/libxl/xl_cmdtable.c Fri Mar 16 15:24:25 2012 +0000
> +++ b/tools/libxl/xl_cmdtable.c Tue Mar 20 15:36:49 2012 +0000
> @@ -31,6 +31,11 @@
> " (deprecated in favour of global -N
> option).\n"
> "-d Enable debug messages.\n"
> "-e Do not wait in the background for the
> death of the domain."
> + "-V, --vncviewer Connect to the VNC display after the
> domain is created.\n"
> + "-A, --vncviewer-autopass\n"
> + " Pass VNC password to viewer via stdin
> and -autopass.\n"
> + "--autopass (xm compatibility)\n"
> +
> },
> { "list",
> &main_list, 0,
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Add vncviewer xm compatibility options the 'xl create' command
2012-03-20 16:05 ` Ian Campbell
@ 2012-03-20 16:43 ` Goncalo Gomes
2012-03-20 16:43 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Goncalo Gomes @ 2012-03-20 16:43 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xen.org
On Tue, 20 Mar 2012, Ian Campbell wrote:
> On Tue, 2012-03-20 at 15:50 +0000, Goncalo Gomes wrote:
> > I've attached the preliminary patch to add vncviewer options to the
> > 'xl create'. It applies cleanly against c/s 4e1d091d10d8. All feedback
> > is welcome.
> >
> > Goncalo
> >
> > # HG changeset patch
> > # User Goncalo Gomes <goncalo.gomes@eu.citrix.com>
> > # Date 1332257809 0
> > # Node ID 46f8afe643dee8de2c592c65204567fbad657616
> > # Parent 4e1d091d10d83130842170cd61f1194e5459f2aa
> > Add vncviewer xm compatibility options the 'xl create' command
>
> Thanks. Are these options actually compatible with "xm create"? Are
I suppose so. Anything specifically that makes you think otherwise?
> you intending to also add the vncviewer option to the config file?
> (I think that was a feature of xm mentioned by the original
> requester of this functionality).
Do you mean having an option like vncviewer=1 in the config file which
would execute the vnc client automatically upon domain creation? If
so, I think someone may already have implemented that, as I noticed it
worked for me, even without adding the said code, but I will do some
more testing around this just to be sure. Do you have a pointer to the
original request?
> Please can you also patch docs/man/xl.pod.1 and/or xl.cfg.pod.5 as
> appropriate.
Yup, will send it along with my second attempt.
> Unfortunately your patch appears to have been whitespace wrapped so it
> won't apply. http://wiki.xen.org/wiki/SubmittingXenPatches contains some
> tips for using "hg email" which can avoid this, otherwise you could try
> Documentation/email-clients.txt from the Linux source for help (I'd give
> you a direct link, but git.kernel.org seems to be down), last resort you
> can attach the file (but also include a copy inline to aid review)
>
> Also please can you add
> [diff]
> showfunc = True
> to ~/.hgrc (it makes function names show up after the @@ lines which
> aids review)
>
> Some review of the code follows.
>
> Thanks,
> Ian.
>
> >
> > Signed-off-by: Goncalo Gomes <goncalo.gomes@eu.citrix.com>
> >
> > diff -r 4e1d091d10d8 -r 46f8afe643de tools/libxl/xl_cmdimpl.c
> > --- a/tools/libxl/xl_cmdimpl.c Fri Mar 16 15:24:25 2012 +0000
> > +++ b/tools/libxl/xl_cmdimpl.c Tue Mar 20 15:36:49 2012 +0000
> > @@ -1347,6 +1347,8 @@
> > int dryrun;
> > int quiet;
> > int console_autoconnect;
> > + int vncviewer;
> > + int vncviewer_autopass;
> > const char *config_file;
> > const char *extra_config; /* extra config string */
> > const char *restore_file;
> > @@ -3306,11 +3308,12 @@
> > int main_create(int argc, char **argv)
> > {
> > const char *filename = NULL;
> > + char *dom = NULL;
> > char *p;
> > char extra_config[1024];
> > struct domain_create dom_info;
> > int paused = 0, debug = 0, daemonize = 1, console_autoconnect =
> > 0,
> > - quiet = 0, monitor = 1;
> > + quiet = 0, monitor = 1, vnc = 1, vncautopass = 1;
> > int opt, rc;
> > int option_index = 0;
> > static struct option long_options[] = {
> > @@ -3318,6 +3321,9 @@
> > {"quiet", 0, 0, 'q'},
> > {"help", 0, 0, 'h'},
> > {"defconfig", 1, 0, 'f'},
> > + {"vncviewer", 0, 0, 'V'},
> > + {"vncviewer-autopass", 0, 0, 'A'},
>
> Here you add 'V' and 'A' but later (in the switch) on you look for 'v'
> and 'a'.
True, I was original using 'v' and 'a', but after re-reading your
email, which suggests '-V', I changed those without much care for the
switch. Good catch!
> It would be useful to have these options for restore too?
>
> > +
>
> Mind the extra whitespace here.
>
> > {0, 0, 0, 0}
> > };
> >
> > @@ -3327,7 +3333,7 @@
> > }
> >
> > while (1) {
> > - opt = getopt_long(argc, argv, "Fhnqf:pcde", long_options,
> > &option_index);
> > + opt = getopt_long(argc, argv, "Fhnqf:pcdeVA", long_options,
> > &option_index);
> > if (opt == -1)
> > break;
> >
> > @@ -3360,6 +3366,12 @@
> > case 'q':
> > quiet = 1;
> > break;
> > break;
> > + case 'v':
> > + vnc = 1;
> > + break;
> > + case 'a':
> > + vnc = vncautopass = 1;
> > + break;
> > default:
> > fprintf(stderr, "option `%c' not supported.\n", optopt);
> > break;
> > @@ -3391,12 +3403,40 @@
> > dom_info.migrate_fd = -1;
> > dom_info.console_autoconnect = console_autoconnect;
> > dom_info.incr_generationid = 0;
> > + dom_info.vncviewer = vnc;
> > + dom_info.vncviewer_autopass = vncautopass;
> > +
> >
> > rc = create_domain(&dom_info);
> > if (rc < 0)
> > return -rc;
> >
> > - return 0;
> > + if (vnc && dryrun) {
> > + printf("vncviewer not being executed for a dryrun\n");
> > + goto out;
> > + }
> > +
> > + if (vnc) {
>
> When you seeded dom_info with the necessary fields I expected that
> create_domain would do this based on those fields, I think that is
> better than doing it here.
So you are suggesting me to move this code inside the create_domain()
path?
> > + dom = libxl_domid_to_name(&ctx, rc);
> > + if (dom) {
> > + rc = vncviewer(dom, vncautopass);
> > + if (!rc) {
> > + goto cleanup;
> > + } else {
> > + rc = 1;
> > + goto out;
>
> Mind your indentation and/or hard tabs please.
Thanks! Will double-check with the next revision.
Goncalo
> > + }
> > + }
> > + }
> > +
> > +cleanup:
> > + if (dom) {
> > + free(dom);
> > + dom = NULL;
> > + }
> > +
> > +out:
> > + return rc;
> > }
> >
> > static void button_press(const char *p, const char *b)
> > diff -r 4e1d091d10d8 -r 46f8afe643de tools/libxl/xl_cmdtable.c
> > --- a/tools/libxl/xl_cmdtable.c Fri Mar 16 15:24:25 2012 +0000
> > +++ b/tools/libxl/xl_cmdtable.c Tue Mar 20 15:36:49 2012 +0000
> > @@ -31,6 +31,11 @@
> > " (deprecated in favour of global -N
> > option).\n"
> > "-d Enable debug messages.\n"
> > "-e Do not wait in the background for the
> > death of the domain."
> > + "-V, --vncviewer Connect to the VNC display after the
> > domain is created.\n"
> > + "-A, --vncviewer-autopass\n"
> > + " Pass VNC password to viewer via stdin
> > and -autopass.\n"
> > + "--autopass (xm compatibility)\n"
> > +
> > },
> > { "list",
> > &main_list, 0,
> >
> >
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Add vncviewer xm compatibility options the 'xl create' command
2012-03-20 16:43 ` Goncalo Gomes
@ 2012-03-20 16:43 ` Ian Campbell
2012-03-20 20:00 ` Goncalo Gomes
0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2012-03-20 16:43 UTC (permalink / raw)
To: Goncalo Gomes; +Cc: xen-devel@lists.xen.org
On Tue, 2012-03-20 at 16:43 +0000, Goncalo Gomes wrote:
> On Tue, 20 Mar 2012, Ian Campbell wrote:
>
> > On Tue, 2012-03-20 at 15:50 +0000, Goncalo Gomes wrote:
> > > I've attached the preliminary patch to add vncviewer options to the
> > > 'xl create'. It applies cleanly against c/s 4e1d091d10d8. All feedback
> > > is welcome.
> > >
> > > Goncalo
> > >
> > > # HG changeset patch
> > > # User Goncalo Gomes <goncalo.gomes@eu.citrix.com>
> > > # Date 1332257809 0
> > > # Node ID 46f8afe643dee8de2c592c65204567fbad657616
> > > # Parent 4e1d091d10d83130842170cd61f1194e5459f2aa
> > > Add vncviewer xm compatibility options the 'xl create' command
> >
> > Thanks. Are these options actually compatible with "xm create"? Are
>
> I suppose so. Anything specifically that makes you think otherwise?
I don't see -A or -V in xend but looking again I do see the long forms,
I guess that's fine.
> > you intending to also add the vncviewer option to the config file?
> > (I think that was a feature of xm mentioned by the original
> > requester of this functionality).
>
> Do you mean having an option like vncviewer=1 in the config file which
> would execute the vnc client automatically upon domain creation? If
> so, I think someone may already have implemented that, as I noticed it
> worked for me, even without adding the said code, but I will do some
> more testing around this just to be sure.
I can't see anything in xl which would do this, are you sure you don't
mean with xm?
> Do you have a pointer to the original request?
it was Jim Burns posting on xen-devel in February, bullet 3 in
http://lists.xen.org/archives/html/xen-devel/2012-02/msg01364.html
[...]
> > When you seeded dom_info with the necessary fields I expected that
> > create_domain would do this based on those fields, I think that is
> > better than doing it here.
>
> So you are suggesting me to move this code inside the create_domain()
> path?
Yes. This should make it trivial to support for xl restore too.
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add vncviewer xm compatibility options the 'xl create' command
2012-03-20 16:43 ` Ian Campbell
@ 2012-03-20 20:00 ` Goncalo Gomes
2012-04-10 10:20 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Goncalo Gomes @ 2012-03-20 20:00 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xen.org
On Tue, 20 Mar 2012, Ian Campbell wrote:
> On Tue, 2012-03-20 at 16:43 +0000, Goncalo Gomes wrote:
> > On Tue, 20 Mar 2012, Ian Campbell wrote:
> >
> > > On Tue, 2012-03-20 at 15:50 +0000, Goncalo Gomes wrote:
> > > > I've attached the preliminary patch to add vncviewer options to the
> > > > 'xl create'. It applies cleanly against c/s 4e1d091d10d8. All feedback
> > > > is welcome.
> > > >
> > > > Goncalo
> > > >
> > > > # HG changeset patch
> > > > # User Goncalo Gomes <goncalo.gomes@eu.citrix.com>
> > > > # Date 1332257809 0
> > > > # Node ID 46f8afe643dee8de2c592c65204567fbad657616
> > > > # Parent 4e1d091d10d83130842170cd61f1194e5459f2aa
> > > > Add vncviewer xm compatibility options the 'xl create' command
> > >
> > > Thanks. Are these options actually compatible with "xm create"? Are
> >
> > I suppose so. Anything specifically that makes you think otherwise?
>
> I don't see -A or -V in xend but looking again I do see the long forms,
> I guess that's fine.
>
> > > you intending to also add the vncviewer option to the config file?
> > > (I think that was a feature of xm mentioned by the original
> > > requester of this functionality).
> >
> > Do you mean having an option like vncviewer=1 in the config file which
> > would execute the vnc client automatically upon domain creation? If
> > so, I think someone may already have implemented that, as I noticed it
> > worked for me, even without adding the said code, but I will do some
> > more testing around this just to be sure.
>
> I can't see anything in xl which would do this, are you sure you don't
> mean with xm?
The problem was that I initialized vnc and vncautopass with a value of
1 when I originally wrote the code, which would always make the
libxl_vncviewer_exec codepath to take place and forgot to change it
back. I will add the support code to the config file to support this
officially.
> > Do you have a pointer to the original request?
>
> it was Jim Burns posting on xen-devel in February, bullet 3 in
> http://lists.xen.org/archives/html/xen-devel/2012-02/msg01364.html
Thanks!
> [...]
> > > When you seeded dom_info with the necessary fields I expected that
> > > create_domain would do this based on those fields, I think that is
> > > better than doing it here.
> >
> > So you are suggesting me to move this code inside the create_domain()
> > path?
>
> Yes. This should make it trivial to support for xl restore too.
I'll move it to that code path.
Goncalo
> Ian.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-10 10:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-20 15:50 [PATCH] Add vncviewer xm compatibility options the 'xl create' command Goncalo Gomes
2012-03-20 16:05 ` Ian Campbell
2012-03-20 16:43 ` Goncalo Gomes
2012-03-20 16:43 ` Ian Campbell
2012-03-20 20:00 ` Goncalo Gomes
2012-04-10 10:20 ` Ian Campbell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).