[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: Support VNC password for QEMU guests



On Wed, Jan 21, 2009 at 01:48:25PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange redhat com> wrote:
> > This patch adds support for using the monitor interface to set the VNC
> > password
> >
> >   (qemu) change vnc password
> >   Password: ********
> >
> > A minor tricky thing is that we can't just send the command and password
> > all in one go, we must wait for the 'Password' prompt before sending the
> > password.
> >
> > When doing this I noticed that virsh dumpxml has no way to request a
> > secure XML dump (required to see the password element), nor did the
> > virsh edit command set the SECURE or INACTIVE flags when changing
> > the XML.
> ...
> > diff --git a/src/qemu_conf.c b/src/qemu_conf.c
> > --- a/src/qemu_conf.c
> > +++ b/src/qemu_conf.c
> > @@ -1138,37 +1138,42 @@ int qemudBuildCommandLine(virConnectPtr
> >
> >      if (vm->def->graphics &&
> >          vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
> > -        char vncdisplay[PATH_MAX];
> > -        int ret;
> > +        virBuffer opt = VIR_BUFFER_INITIALIZER;
> > +        char *optstr;
> ...
> > +        optstr = virBufferContentAndReset(&opt);
> >
> >          ADD_ARG_LIT("-vnc");
> > -        ADD_ARG_LIT(vncdisplay);
> > +        ADD_ARG(optstr);
> 
> Looks fine.
> Just be sure to add this:
> 
>            VIR_FREE(optstr);

No need - ADD_ARG() owns the pointer - ADD_ARG_LIT() strdups, hence why
I changed it.

> BTW, can you outline what you do to test this?
> and I'll see about adding something to exercise the new code.

There's two parts to testing
 
 - The ARGV for QEMU - trivially to add more datafiles to qemuxml2argvtest.c
 - The monitor interaction

We have no way to testing monitor interaction, and definitely do not want
to be in the business of running real QEMU instances to test this. What
I think we should do is to create a fake / mock QEMU binary in the tests
directory, which simulates the monitor console. It would output pre-defined
responses for each of the monitor commands we care about. This would let
us unit test each of the individual functions that process monitor commands
in isolation. It actally might be best to just use a self-pipe in the test
case and have a thread to provide the server end of the monitor - that
would let us trivially tell it what response we want to get back to each
command we're about to test.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]