[virt-tools-list] [PATCH virt-viewer 3/3] virt-viewer-main: exit when domain names doesn't match

Daniel P. Berrange berrange at redhat.com
Tue Apr 14 14:00:36 UTC 2015


On Tue, Apr 14, 2015 at 08:55:49AM -0500, Jonathon Jongsma wrote:
> On Tue, 2015-04-14 at 14:08 +0200, Pavel Grunt wrote:
> > Since the domain name is required as a parameter for the '--wait'
> > option (commit a830275344c88aef12166661b68ea2b4429c7212 ), it is
> > neccessary to check whether all domains names are the same. Otherwise
> > it wouldn't be clear which name should be used.
> > 
> > related: rhbz#1211573
> > ---
> >  src/virt-viewer-main.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/src/virt-viewer-main.c b/src/virt-viewer-main.c
> > index 8c1ca80..b2ae0be 100644
> > --- a/src/virt-viewer-main.c
> > +++ b/src/virt-viewer-main.c
> > @@ -108,6 +108,11 @@ int main(int argc, char **argv)
> >          goto cleanup;
> >      }
> >  
> > +    if (args && waitvm != NULL && g_strcmp0(waitvm, args[0])) {
> > +        g_printerr(_("\nDomain names mismatch, got '%s' and '%s'\n\n%s\n\n"), waitvm, args[0], help_msg);
> > +        goto cleanup;
> > +    }
> > +
> >      viewer = virt_viewer_new(uri, (args) ? args[0] : waitvm, direct, attach, waitvm != NULL, reconnect);
> >      if (viewer == NULL)
> >          goto cleanup;
> 
> 
> I don't remember noticing the change that required an argument for the
> --wait switch. I'm not sure that's a great idea. It breaks the
> command-line "API", and (as this commit shows) it requires the user to
> specify the same domain twice on the same command-line, which requires
> additional sanity-checking. This seems quite awkward. Is it really
> necessary?

Yeah, this is very bad. We cannot change the semantics of the pre-existing
--wait switch as that will break any apps / scripts that are using it, such
as virt-install.

We must revert this change:

  commit a830275344c88aef12166661b68ea2b4429c7212
  Author: Pavel Grunt <pgrunt at redhat.com>
  Date:   Tue Apr 7 11:06:46 2015 +0200

    virt-viewer-main: Require domain name as argument for '--wait'


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|




More information about the virt-tools-list mailing list