Re: [PATCH] Add vncviewer xm compatibility options the 'xl create' command

[Goncalo Gomes <Goncalo.Gomes@EU.CITRIX.COM>]

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.
>