[libvirt] [RFC] [libvirt-gconfig] Suggestion about (maybe) re-factoring GVirtConfigDomainGraphics

[Daniel P. Berrange]

On Mon, Feb 29, 2016 at 11:51:26PM +0100, Fabiano Fidêncio wrote:
> Howdy!
> 
> I've been trying to use libvirt-gobject and libvirt-gconfig, on
> virt-viewer, for accessing VMs and looking at their config, instead of
> using libvrit and parsing XML directly and turns out, that
> libvirt-gconfig is not exactly handful for a "read-only" use case (as
> virt-viewer's one).
> 
> Let me try to explain pointing to some code as example and then I'll
> give my suggestions.
> For example, let's take a look on
> https://git.fedorahosted.org/cgit/virt-viewer.git/tree/src/virt-viewer.c#n468
> 
> In this function, the first thing done is to get the type of the
> graphic device (SPICE or VNC) and here is the first problem. There is
> no straightforward way for doing this on libvirt-gconfig (or is
> there?).
> Seems easier to continue getting the type using libxml and then use
> the specific spice/vnc functions for getting the properties. And here
> is the second problem, because I'll always need to have an if/else
> statement for getting the properties. Something like:
> if (g_str_equal(type, "vnc"))
>   port = gvir_config_domain_graphics_vnc_get_port(domain);
> else if (g_str_equal(type, "spice"))
>   port = gvir_config_domain_graphics_spice_get_port(domain);
> 
> This kind of usage makes me think that libvirt-gconfig is missing some
> abstraction class, parent of GVirConfigDomainGraphics{Spice,Vnc,...)
> which could provide virtual functions like:
>   gtype = gvir_config_domain_graphics_get_gtype(domain);
>   port = gvir_config_domain_graphics_get_port(domain);
> 
> Thinking a bit about it and taking a look in the
> GVirConfigDomainGraphics code, is possible to see a possibility for 2
> new classes:
> GVirConfigDomainGraphicsLocal and GVirConfigDomainGraphicsRemote.
> 
> Then we could have something like:
> GVirtConfigDomainGraphics
>  |_
>  |   GVirtConfigDomainGraphicsLocal
>  |   |_
>  |   |  GVirtConfigDomainGraphicsLocalDesktop
>  |   |_
>  |      GVirtConfigDomainGraphicsLocalSdl
>  |_
>      GVirtConfigDomainGraphicsRemote
>      |_
>      |   GVirtConfigDomainGraphicsRemoteSpice
>      |_
>      |   GVirtConfigDomainGraphicsRemoteVnc
>      |_
>          GVirtConfigDomainGraphicsRemoteRdp
> 
> I do know that Local and Remote are not exactly accurate names, but is
> the best that I could come up with.
> 
> So, would be acceptable to introduce these two new classes and then
> have the specific graphics classes inheriting from those? Does this
> make sense for you, people?
> 
> I'm more than happy in provide the code for this, but not before we
> discuss and set a decision about the approach. :-)

As long as you can introduce the new classes without breaking API
compatability of existing classes, I think the idea is reasonable.


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