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@xxxxxxxxxx> 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
Attachment:
pgp7JeGrC0fWp.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list