[libvirt] [libvirt-glib 33/37] Implement gvir_config_device_chardev_set_source

[Christophe Fergeau]

On Sat, Nov 12, 2011 at 12:00:33AM +0100, Marc-André Lureau wrote:
> On Thu, Nov 10, 2011 at 9:34 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > +    source_type = gvir_config_chardev_source_class_get_nick(G_OBJECT_TYPE(source));
> >
> 
> I don't see why this is necessarily a class method, I virtual method
> would have the same result (but simpler to use).

A virtual method isn't really a good fit here since this nick really has to
be the same for all instances of the same class. Imo it doesn't make much
sense to have an object as a parameter (which will be needed
to resolve the vfunc) while the function will return something or set
something that is not specific to that particular object, but that is a
common property of all the objects instanciating this class.

> 
> I don't understand the design of it either. Why not have the chardev
> type as a chardev property? Why should the child node set it when it
> is parented?

Yes chardevs are a tricky, it took me a bit of time to decide how I wanted
to handle them.
If you look at http://libvirt.org/formatdomain.html#elementsConsole , it
starts by describing the different guest interfaces that are available
(serial, parallel, ...) with (generally) an associated <target> attribute.
The name of the chardev XML node corresponds to the guest device we want to
define (serial, parallel, ...).

Then the doc above describes the host interfaces. The host interface type
is set with a "type" attribute on the root node. Then depending on this
type attribute, we have a different set of possible formats for the
<source> node.

This is why I coupled the node name with the target attribute, and the type
with the source attribute. A chardev device is really a (guest interface,
host interface) couple, and most combinations are possible, that's why I
chose to build the GVirConfigDeviceChardevNode by combining a
GVirConfigChardevSource and a GVirConfigChardevTarget.

However, your question and rereading the doc to answer you made me think
more about it
* it's too much of a simplification to assume the host interface will be
  defined by a single source node, it can go with a <protocol> node, and
  there can be several <source> node (see type="udp" and type="tcp")
* something I'm not happy with in the current design is that it's really
  weird to have an empty GVirConfigDeviceChardev node, the xml node can't
  even have a name since it will only be known when we set a
  GVirConfigChardevTarget for it

Maybe GVirConfigDeviceChardev should be abstract, and we should have
GVirConfigDeviceChardevSerial, GVirConfigDeviceChardevConsole, ...
And I'll have to think more about the first point...

Hope that helps,

Christophe

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111115/6ff865b8/attachment-0001.sig>