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

[Christophe Fergeau]

Hey,

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);

For what it's worth, you could cheat by adding some gobject properties
on both classes, and do
g_object_get(G_OBJECT(graphics), "port", &port, NULL); and expect 2
properties with the same name to be defined in the 2 classes.
I agree this is not a great suggestion though ;)

> 
> 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);

I'd prefer to avoid this one, it's not much different from what you can
already achieve with g_type_name (G_OBJECT_TYPE (graphics)), or even
better, with
GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE(graphics) / GVIR_CONFIG_IS_DOMAIN_GRAPHICS_VNC(graphics)

As this is a bit low-level, we could have some helpers to avoid doing
these for common cases, but in this case, I'd expect something higher
level than _get_gtype().

You mention "virtual functions", if by that you mean function pointers
in the parent class which are overriden by the vnc/spice child classes,
I'd try to avoid them if possible, and move the (hopefully) duplicate
logic in the child classes toward the parent class, and keep API/ABI by
just having

int
gvir_config_domain_graphics_spice_get_port(GVirConfigDomainGraphicsSpice *graphics)
{
    return gvir_config_domain_graphics_get_port(GVIR_CONFIG_DOMAIN_GRAPHICS_REMOTE(graphics));
}

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160301/57112bfb/attachment-0001.sig>