Re: [libvirt] [libvirt-glib 1/3] Add GVirConfigDomainHostdev

Hey, A gconfig: prefix in the commit shortlog would be nice

On Thu, Jan 28, 2016 at 04:32:12PM +0100, Zeeshan Ali (Khattak) wrote: > diff --git a/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c b/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c > new file mode 100644 > index 0000000..42eb184 > --- /dev/null > +++ b/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c > @@ -0,0 +1,180 @@ > +/* > + * libvirt-gconfig-domain-hostdev.c: libvirt domain hostdev configuration > + * > + * Copyright (C) 2012 Red Hat, Inc. You can set this to 2012-2016 > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>;. > + * > + * Authors: Zeeshan Ali (Khattak) <zeeshanak(a)gnome.org&gt; > + * Christophe Fergeau <cfergeau(a)redhat.com&gt; > + */ > + > +#include <config.h> > + > +#include "libvirt-gconfig/libvirt-gconfig.h" > +#include "libvirt-gconfig/libvirt-gconfig-private.h" > + > +#define GVIR_CONFIG_DOMAIN_HOSTDEV_GET_PRIVATE(obj) \ > + (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV, GVirConfigDomainHostdevPrivate)) > + > +struct _GVirConfigDomainHostdevPrivate > +{ > + gboolean unused; > +}; > + > +G_DEFINE_ABSTRACT_TYPE(GVirConfigDomainHostdev, gvir_config_domain_hostdev, GVIR_CONFIG_TYPE_DOMAIN_DEVICE); > + > + > +static void gvir_config_domain_hostdev_class_init(GVirConfigDomainHostdevClass *klass) > +{ > + g_type_class_add_private(klass, sizeof(GVirConfigDomainHostdevPrivate)); > +} > + > + > +static void gvir_config_domain_hostdev_init(GVirConfigDomainHostdev *hostdev) > +{ > + hostdev->priv = GVIR_CONFIG_DOMAIN_HOSTDEV_GET_PRIVATE(hostdev); > +} > + > +G_GNUC_INTERNAL GVirConfigDomainDevice * > +gvir_config_domain_hostdev_new_from_tree(GVirConfigXmlDoc *doc, > + xmlNodePtr tree) > +{ > + const char *type; > + GType gtype; > + > + type = gvir_config_xml_get_attribute_content(tree, "type"); > + if (type == NULL) > + return NULL; > + > + if (g_str_equal(type, "usb")) { > + goto unimplemented; > + } else if (g_str_equal(type, "pci")) { > + goto unimplemented; > + } else if (g_str_equal(type, "scsi")) { > + goto unimplemented; > + } else { > + g_debug("Unknown domain hostdev node: %s", type); > + return NULL; > + } > + > + return GVIR_CONFIG_DOMAIN_DEVICE(gvir_config_object_new_from_tree(gtype, doc, NULL, tree)); > + > +unimplemented: > + g_debug("Parsing of '%s' domain hostdev nodes is unimplemented", type); > + return NULL; > +} > + > +void gvir_config_domain_hostdev_set_boot_order(GVirConfigDomainHostdev *hostdev, > + gint order) > +{ > + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV(hostdev)); > + > + if (order >= 0) { > + char *order_str = g_strdup_printf("%u", order); > + > + gvir_config_object_replace_child_with_attribute(GVIR_CONFIG_OBJECT(hostdev), > + "boot", > + "order", > + order_str); > + g_free(order_str); > + } else { > + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(hostdev), > + "boot", > + NULL); > + } > +} > + > +gint gvir_config_domain_hostdev_get_boot_order(GVirConfigDomainHostdev *hostdev) > +{ > + xmlNodePtr hostdev_node; > + xmlNodePtr boot_node; > + const char *order_str; > + char *end; > + guint order; > + > + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV(hostdev), -1); > + > + hostdev_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(hostdev)); > + g_return_val_if_fail(hostdev_node != NULL, -1); > + > + boot_node = gvir_config_xml_get_element(hostdev_node, "boot", NULL); > + if (boot_node == NULL) > + return -1; > + > + order_str = gvir_config_xml_get_attribute_content(boot_node, "order"); > + g_return_val_if_fail(order_str != NULL, -1); > + > + order = strtoul(order_str, &end, 0); > + g_return_val_if_fail(*end == '\0', -1); > + > + return order; This can be replaced with a call to gvir_config_object_get_attribute_uint64 (I have a patch doing that, and a few additional ones, I'll send a v2 series with your patches+mine). > +} > + > +void gvir_config_domain_hostdev_set_readonly(GVirConfigDomainHostdev *hostdev, > + gboolean readonly) > +{ > + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV(hostdev)); > + > + if (readonly) { > + GVirConfigObject *node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(hostdev), "readonly"); I'd split this long line > + g_object_unref(node); > + } else { > + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(hostdev), "readonly", NULL); > + } > +} > + > +gboolean gvir_config_domain_hostdev_get_readonly(GVirConfigDomainHostdev *hostdev) > +{ > + xmlNodePtr hostdev_node; > + xmlNodePtr ro_node; > + > + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV(hostdev), FALSE); > + > + hostdev_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(hostdev)); > + g_return_val_if_fail(hostdev_node != NULL, FALSE); > + > + ro_node = gvir_config_xml_get_element(hostdev_node, "readonly", NULL); > + > + return (ro_node != NULL); > +} libvirt XML documentation says that readonly and shareable are only supported by SCSI devices, should this be in the base class, or only in the SCSI child class?