4:51 p.m.
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#...
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. :-)
I'm looking forward for some feedback.
--
Fabiano Fidêncio
3:50 a.m.
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#...
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 :|
8:39 a.m.
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#...
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
3069
days inactive
3070
days old
2 comments
3 participants
participants (3)
-
Christophe Fergeau -
Daniel P. Berrange -
Fabiano Fidêncio