[libvirt] [libvirt-glib 1/3] Add GVirConfigDomainHostdev
Add API to read and write domain/devices/hostdev nodes. This patch only adds the baseclass and hence is not useful on it's own. A more specific subclass to represent PCI devices will be added in a following patch. --- libvirt-gconfig/Makefile.am | 2 + .../libvirt-gconfig-domain-device-private.h | 3 + libvirt-gconfig/libvirt-gconfig-domain-device.c | 2 +- libvirt-gconfig/libvirt-gconfig-domain-hostdev.c | 180 +++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain-hostdev.h | 76 +++++++++ libvirt-gconfig/libvirt-gconfig.h | 1 + libvirt-gconfig/libvirt-gconfig.sym | 11 ++ 7 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-hostdev.c create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-hostdev.h diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am index 77b2032..4294bab 100644 --- a/libvirt-gconfig/Makefile.am +++ b/libvirt-gconfig/Makefile.am @@ -50,6 +50,7 @@ GCONFIG_HEADER_FILES = \ libvirt-gconfig-domain-graphics-sdl.h \ libvirt-gconfig-domain-graphics-spice.h \ libvirt-gconfig-domain-graphics-vnc.h \ + libvirt-gconfig-domain-hostdev.h \ libvirt-gconfig-domain-input.h \ libvirt-gconfig-domain-interface.h \ libvirt-gconfig-domain-interface-bridge.h \ @@ -141,6 +142,7 @@ GCONFIG_SOURCE_FILES = \ libvirt-gconfig-domain-graphics-sdl.c \ libvirt-gconfig-domain-graphics-spice.c \ libvirt-gconfig-domain-graphics-vnc.c \ + libvirt-gconfig-domain-hostdev.c \ libvirt-gconfig-domain-input.c \ libvirt-gconfig-domain-interface.c \ libvirt-gconfig-domain-interface-bridge.c \ diff --git a/libvirt-gconfig/libvirt-gconfig-domain-device-private.h b/libvirt-gconfig/libvirt-gconfig-domain-device-private.h index 062c0e2..c45e1df 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-device-private.h +++ b/libvirt-gconfig/libvirt-gconfig-domain-device-private.h @@ -43,6 +43,9 @@ GVirConfigDomainDevice * gvir_config_domain_graphics_new_from_tree(GVirConfigXmlDoc *doc, xmlNodePtr tree); GVirConfigDomainDevice * +gvir_config_domain_hostdev_new_from_tree(GVirConfigXmlDoc *doc, + xmlNodePtr tree); +GVirConfigDomainDevice * gvir_config_domain_interface_new_from_tree(GVirConfigXmlDoc *doc, xmlNodePtr tree); GVirConfigDomainDevice * diff --git a/libvirt-gconfig/libvirt-gconfig-domain-device.c b/libvirt-gconfig/libvirt-gconfig-domain-device.c index 3d2b9b3..8a75cea 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-device.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-device.c @@ -66,7 +66,7 @@ gvir_config_domain_device_new_from_tree(GVirConfigXmlDoc *doc, } else if (xmlStrEqual(tree->name, (xmlChar*)"lease")) { goto unimplemented; } else if (xmlStrEqual(tree->name, (xmlChar*)"hostdev")) { - goto unimplemented; + return gvir_config_domain_hostdev_new_from_tree(doc, tree); } else if (xmlStrEqual(tree->name, (xmlChar*)"redirdev")) { type = GVIR_CONFIG_TYPE_DOMAIN_REDIRDEV; } else if (xmlStrEqual(tree->name, (xmlChar*)"smartcard")) { 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. + * + * 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@gnome.org> + * Christophe Fergeau <cfergeau@redhat.com> + */ + +#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; +} + +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"); + 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); +} + +void gvir_config_domain_hostdev_set_shareable(GVirConfigDomainHostdev *hostdev, + gboolean shareable) +{ + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV(hostdev)); + + if (shareable) { + GVirConfigObject *node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(hostdev), "shareable"); + g_object_unref(node); + } else { + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(hostdev), "shareable", NULL); + } +} + +gboolean gvir_config_domain_hostdev_get_shareable(GVirConfigDomainHostdev *hostdev) +{ + xmlNodePtr hostdev_node; + xmlNodePtr shareable_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); + + shareable_node = gvir_config_xml_get_element(hostdev_node, "shareable", NULL); + + return (shareable_node != NULL); +} diff --git a/libvirt-gconfig/libvirt-gconfig-domain-hostdev.h b/libvirt-gconfig/libvirt-gconfig-domain-hostdev.h new file mode 100644 index 0000000..b26100f --- /dev/null +++ b/libvirt-gconfig/libvirt-gconfig-domain-hostdev.h @@ -0,0 +1,76 @@ +/* + * libvirt-gconfig-domain-hostdev.h: libvirt domain hostdev configuration + * + * Copyright (C) 2012 Red Hat, Inc. + * + * 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@gnome.org> + * Christophe Fergeau <cfergeau@redhat.com> + */ + +#if !defined(__LIBVIRT_GCONFIG_H__) && !defined(LIBVIRT_GCONFIG_BUILD) +#error "Only <libvirt-gconfig/libvirt-gconfig.h> can be included directly." +#endif + +#ifndef __LIBVIRT_GCONFIG_DOMAIN_HOSTDEV_H__ +#define __LIBVIRT_GCONFIG_DOMAIN_HOSTDEV_H__ + +G_BEGIN_DECLS + +#define GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV (gvir_config_domain_hostdev_get_type ()) +#define GVIR_CONFIG_DOMAIN_HOSTDEV(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV, GVirConfigDomainHostdev)) +#define GVIR_CONFIG_DOMAIN_HOSTDEV_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV, GVirConfigDomainHostdevClass)) +#define GVIR_CONFIG_IS_DOMAIN_HOSTDEV(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV)) +#define GVIR_CONFIG_IS_DOMAIN_HOSTDEV_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV)) +#define GVIR_CONFIG_DOMAIN_HOSTDEV_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV, GVirConfigDomainHostdevClass)) + +typedef struct _GVirConfigDomainHostdev GVirConfigDomainHostdev; +typedef struct _GVirConfigDomainHostdevPrivate GVirConfigDomainHostdevPrivate; +typedef struct _GVirConfigDomainHostdevClass GVirConfigDomainHostdevClass; + +struct _GVirConfigDomainHostdev +{ + GVirConfigDomainDevice parent; + + GVirConfigDomainHostdevPrivate *priv; + + /* Do not add fields to this struct */ +}; + +struct _GVirConfigDomainHostdevClass +{ + GVirConfigDomainDeviceClass parent_class; + + gpointer padding[20]; +}; + +GType gvir_config_domain_hostdev_get_type(void); + +void gvir_config_domain_hostdev_set_boot_order(GVirConfigDomainHostdev *hostdev, + gint order); +gint gvir_config_domain_hostdev_get_boot_order(GVirConfigDomainHostdev *hostdev); + +void gvir_config_domain_hostdev_set_readonly(GVirConfigDomainHostdev *hostdev, + gboolean readonly); +gboolean gvir_config_domain_hostdev_get_readonly(GVirConfigDomainHostdev *hostdev); + +void gvir_config_domain_hostdev_set_shareable(GVirConfigDomainHostdev *hostdev, + gboolean shareable); +gboolean gvir_config_domain_hostdev_get_shareable(GVirConfigDomainHostdev *hostdev); + +G_END_DECLS + +#endif /* __LIBVIRT_GCONFIG_DOMAIN_HOSTDEV_H__ */ diff --git a/libvirt-gconfig/libvirt-gconfig.h b/libvirt-gconfig/libvirt-gconfig.h index 4624003..cfa9dd3 100644 --- a/libvirt-gconfig/libvirt-gconfig.h +++ b/libvirt-gconfig/libvirt-gconfig.h @@ -67,6 +67,7 @@ #include <libvirt-gconfig/libvirt-gconfig-domain-graphics-sdl.h> #include <libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.h> #include <libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.h> +#include <libvirt-gconfig/libvirt-gconfig-domain-hostdev.h> #include <libvirt-gconfig/libvirt-gconfig-domain-input.h> #include <libvirt-gconfig/libvirt-gconfig-domain-interface.h> #include <libvirt-gconfig/libvirt-gconfig-domain-interface-bridge.h> diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index 89dd589..0a12ce4 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -733,4 +733,15 @@ global: gvir_config_domain_video_set_vgamem; } LIBVIRT_GCONFIG_0.2.1; +LIBVIRT_GCONFIG_0.2.3 { +global: + gvir_config_domain_hostdev_get_boot_order; + gvir_config_domain_hostdev_get_readonly; + gvir_config_domain_hostdev_get_shareable; + gvir_config_domain_hostdev_get_type; + gvir_config_domain_hostdev_set_boot_order; + gvir_config_domain_hostdev_set_readonly; + gvir_config_domain_hostdev_set_shareable; +} LIBVIRT_GCONFIG_0.2.2; + # .... define new API here using predicted next version number .... -- 2.5.0
Add API to read and write PCI hostdev nodes. --- libvirt-gconfig/Makefile.am | 2 + .../libvirt-gconfig-domain-hostdev-pci.c | 211 +++++++++++++++++++++ .../libvirt-gconfig-domain-hostdev-pci.h | 80 ++++++++ libvirt-gconfig/libvirt-gconfig-domain-hostdev.c | 2 +- libvirt-gconfig/libvirt-gconfig.h | 1 + libvirt-gconfig/libvirt-gconfig.sym | 9 + 6 files changed, 304 insertions(+), 1 deletion(-) create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.c create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.h diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am index 4294bab..245313d 100644 --- a/libvirt-gconfig/Makefile.am +++ b/libvirt-gconfig/Makefile.am @@ -51,6 +51,7 @@ GCONFIG_HEADER_FILES = \ libvirt-gconfig-domain-graphics-spice.h \ libvirt-gconfig-domain-graphics-vnc.h \ libvirt-gconfig-domain-hostdev.h \ + libvirt-gconfig-domain-hostdev-pci.h \ libvirt-gconfig-domain-input.h \ libvirt-gconfig-domain-interface.h \ libvirt-gconfig-domain-interface-bridge.h \ @@ -143,6 +144,7 @@ GCONFIG_SOURCE_FILES = \ libvirt-gconfig-domain-graphics-spice.c \ libvirt-gconfig-domain-graphics-vnc.c \ libvirt-gconfig-domain-hostdev.c \ + libvirt-gconfig-domain-hostdev-pci.c \ libvirt-gconfig-domain-input.c \ libvirt-gconfig-domain-interface.c \ libvirt-gconfig-domain-interface-bridge.c \ diff --git a/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.c b/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.c new file mode 100644 index 0000000..ed1d146 --- /dev/null +++ b/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.c @@ -0,0 +1,211 @@ +/* + * libvirt-gconfig-domain-hostdev.c: libvirt domain hostdev configuration + * + * Copyright (C) 2012 Red Hat, Inc. + * + * 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@gnome.org> + * Christophe Fergeau <cfergeau@redhat.com> + */ + +#include <config.h> + +#include "libvirt-gconfig/libvirt-gconfig.h" +#include "libvirt-gconfig/libvirt-gconfig-private.h" + +#define GVIR_CONFIG_DOMAIN_HOSTDEV_PCI_GET_PRIVATE(obj) \ + (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, GVirConfigDomainHostdevPciPrivate)) + +struct _GVirConfigDomainHostdevPciPrivate +{ + gboolean unused; +}; + +G_DEFINE_TYPE(GVirConfigDomainHostdevPci, gvir_config_domain_hostdev_pci, GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV); + +static void gvir_config_domain_hostdev_pci_class_init(GVirConfigDomainHostdevPciClass *klass) +{ + g_type_class_add_private(klass, sizeof(GVirConfigDomainHostdevPciPrivate)); +} + + +static void gvir_config_domain_hostdev_pci_init(GVirConfigDomainHostdevPci *hostdev) +{ + hostdev->priv = GVIR_CONFIG_DOMAIN_HOSTDEV_PCI_GET_PRIVATE(hostdev); +} + +/** + * gvir_config_domain_hostdev_pci_new: + * + * Creates a new #GVirConfigDomainHostdevPci with a reference count of 1. + * + * Returns: a new #GVirConfigDomainHostdevPci + */ +GVirConfigDomainHostdevPci *gvir_config_domain_hostdev_pci_new(void) +{ + GVirConfigObject *object; + + object = gvir_config_object_new(GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, + "hostdev", NULL); + gvir_config_object_set_attribute(object, "mode", "subsystem", NULL); + gvir_config_object_set_attribute(object, "type", "pci", NULL); + + return GVIR_CONFIG_DOMAIN_HOSTDEV_PCI(object); +} + +/** + * gvir_config_domain_hostdev_pci_new_from_xml: + * @xml: xml data to create the host device from + * @error: return location for a #GError, or NULL + * + * Creates a new #GVirConfigDomainHostdevPci with a reference count of 1. + * The host device object will be created using the XML description stored + * in @xml. This is a fragment of libvirt domain XML whose root node is + * <hostdev>. + * + * Returns: a new #GVirConfigDomainHostdevPci, or NULL if @xml failed to + * be parsed. + */ +GVirConfigDomainHostdevPci *gvir_config_domain_hostdev_pci_new_from_xml(const gchar *xml, + GError **error) +{ + GVirConfigObject *object; + + object = gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, + "hostdev", NULL, xml, error); + if (*error != NULL) + return NULL; + + if (g_strcmp0(gvir_config_object_get_attribute(object, NULL, "type"), "pci") != 0) { + g_object_unref(G_OBJECT(object)); + g_return_val_if_reached(NULL); + } + + return GVIR_CONFIG_DOMAIN_HOSTDEV_PCI(object); +} + +void gvir_config_domain_hostdev_pci_set_address(GVirConfigDomainHostdevPci *hostdev, + GVirConfigDomainAddressPci *address) +{ + GVirConfigObject *source; + GVirConfigObject *addr_object; + xmlNodePtr node; + xmlAttrPtr attr; + + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev)); + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_ADDRESS_PCI(address)); + addr_object = GVIR_CONFIG_OBJECT(address); + node = gvir_config_object_get_xml_node(addr_object); + g_return_if_fail(node != NULL); + + source = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(hostdev), + "source"); + /* We can't just use GVirConfigDomainAddressPci's node, as is, since it + * contains a 'type' attribute that's not valid in this context. So we + * create a copy for our use and just delete the 'type' node from it. + */ + node = xmlCopyNode(node, 1); + for (attr = node->properties; attr; attr = attr->next) { + if (g_strcmp0 ("type", (char *)attr->name) == 0) { + xmlRemoveProp (attr); + break; + } + } + gvir_config_object_set_child(source, node); +} + +GVirConfigDomainAddressPci *gvir_config_domain_hostdev_pci_get_address(GVirConfigDomainHostdevPci *hostdev) +{ + GVirConfigObject *source; + GVirConfigObject* address; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev), NULL); + + source = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(hostdev), "source"); + if (source == NULL) + return NULL; + + address = gvir_config_object_get_child_with_type(source, + "address", + GVIR_CONFIG_TYPE_DOMAIN_ADDRESS_PCI); + return GVIR_CONFIG_DOMAIN_ADDRESS_PCI(address); +} + +void gvir_config_domain_hostdev_pci_set_managed(GVirConfigDomainHostdevPci *hostdev, + gboolean managed) +{ + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev)); + + gvir_config_object_set_attribute(GVIR_CONFIG_OBJECT(hostdev), + "managed", + managed? "yes": "no", + NULL); +} + +gboolean gvir_config_domain_hostdev_pci_get_managed(GVirConfigDomainHostdevPci *hostdev) +{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev), FALSE); + + return gvir_config_object_get_attribute_boolean(GVIR_CONFIG_OBJECT(hostdev), + NULL, + "managed", + FALSE); +} + +void gvir_config_domain_hostdev_pci_set_rom(GVirConfigDomainHostdevPci *hostdev, + const gchar *rom_file, + gboolean bar) +{ + GVirConfigObject *rom; + xmlNodePtr rom_node; + + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev)); + + rom = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(hostdev), "rom"); + rom_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(rom)); + + xmlSetProp(rom_node, + (const xmlChar *) "bar", + bar? (const xmlChar *) "on" : (const xmlChar *) "off"); + xmlSetProp(rom_node, + (const xmlChar *) "file", + (const xmlChar *) rom_file); +} + +const gchar *gvir_config_domain_hostdev_pci_get_rom(GVirConfigDomainHostdevPci *hostdev, + gboolean *bar) +{ + xmlNodePtr hostdev_node; + xmlNodePtr rom_node; + const gchar *bar_str; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev), NULL); + + hostdev_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(hostdev)); + g_return_val_if_fail(hostdev_node != NULL, NULL); + + rom_node = gvir_config_xml_get_element(hostdev_node, "rom", NULL); + if (!rom_node || !(rom_node->children)) + return NULL; + + bar_str = gvir_config_xml_get_attribute_content(rom_node, "bar"); + if (g_strcmp0(bar_str, "on")) + *bar = TRUE; + else + *bar = FALSE; + + return (const char *) rom_node->children->content; +} diff --git a/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.h b/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.h new file mode 100644 index 0000000..e03681e --- /dev/null +++ b/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.h @@ -0,0 +1,80 @@ +/* + * libvirt-gconfig-domain-hostdev.h: libvirt domain hostdev configuration + * + * Copyright (C) 2012 Red Hat, Inc. + * + * 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@gnome.org> + * Christophe Fergeau <cfergeau@redhat.com> + */ + +#if !defined(__LIBVIRT_GCONFIG_H__) && !defined(LIBVIRT_GCONFIG_BUILD) +#error "Only <libvirt-gconfig/libvirt-gconfig.h> can be included directly." +#endif + +#ifndef __LIBVIRT_GCONFIG_DOMAIN_HOSTDEV_PCI_H__ +#define __LIBVIRT_GCONFIG_DOMAIN_HOSTDEV_PCI_H__ + +G_BEGIN_DECLS + +#define GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI (gvir_config_domain_hostdev_pci_get_type ()) +#define GVIR_CONFIG_DOMAIN_HOSTDEV_PCI(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, GVirConfigDomainHostdevPci)) +#define GVIR_CONFIG_DOMAIN_HOSTDEV_PCI_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, GVirConfigDomainHostdevPciClass)) +#define GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI)) +#define GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI)) +#define GVIR_CONFIG_DOMAIN_HOSTDEV_PCI_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, GVirConfigDomainHostdevPciClass)) + +typedef struct _GVirConfigDomainHostdevPci GVirConfigDomainHostdevPci; +typedef struct _GVirConfigDomainHostdevPciPrivate GVirConfigDomainHostdevPciPrivate; +typedef struct _GVirConfigDomainHostdevPciClass GVirConfigDomainHostdevPciClass; + +struct _GVirConfigDomainHostdevPci +{ + GVirConfigDomainHostdev parent; + + GVirConfigDomainHostdevPciPrivate *priv; + + /* Do not add fields to this struct */ +}; + +struct _GVirConfigDomainHostdevPciClass +{ + GVirConfigDomainHostdevClass parent_class; + + gpointer padding[20]; +}; + +GType gvir_config_domain_hostdev_pci_get_type(void); + +GVirConfigDomainHostdevPci *gvir_config_domain_hostdev_pci_new(void); +GVirConfigDomainHostdevPci *gvir_config_domain_hostdev_pci_new_from_xml(const gchar *xml, + GError **error); +void gvir_config_domain_hostdev_pci_set_address(GVirConfigDomainHostdevPci *hostdev, + GVirConfigDomainAddressPci *address); +GVirConfigDomainAddressPci *gvir_config_domain_hostdev_pci_get_address(GVirConfigDomainHostdevPci *hostdev); + +void gvir_config_domain_hostdev_pci_set_managed(GVirConfigDomainHostdevPci *hostdev, + gboolean managed); +gboolean gvir_config_domain_hostdev_pci_get_managed(GVirConfigDomainHostdevPci *hostdev); +void gvir_config_domain_hostdev_pci_set_rom(GVirConfigDomainHostdevPci *hostdev, + const gchar *rom_file, + gboolean bar); +const gchar *gvir_config_domain_hostdev_pci_get_rom(GVirConfigDomainHostdevPci *hostdev, + gboolean *bar); + +G_END_DECLS + +#endif /* __LIBVIRT_GCONFIG_DOMAIN_HOSTDEV_PCI_H__ */ diff --git a/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c b/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c index 42eb184..b06b1ee 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c @@ -62,7 +62,7 @@ gvir_config_domain_hostdev_new_from_tree(GVirConfigXmlDoc *doc, if (g_str_equal(type, "usb")) { goto unimplemented; } else if (g_str_equal(type, "pci")) { - goto unimplemented; + gtype = GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI; } else if (g_str_equal(type, "scsi")) { goto unimplemented; } else { diff --git a/libvirt-gconfig/libvirt-gconfig.h b/libvirt-gconfig/libvirt-gconfig.h index cfa9dd3..6462154 100644 --- a/libvirt-gconfig/libvirt-gconfig.h +++ b/libvirt-gconfig/libvirt-gconfig.h @@ -68,6 +68,7 @@ #include <libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.h> #include <libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.h> #include <libvirt-gconfig/libvirt-gconfig-domain-hostdev.h> +#include <libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.h> #include <libvirt-gconfig/libvirt-gconfig-domain-input.h> #include <libvirt-gconfig/libvirt-gconfig-domain-interface.h> #include <libvirt-gconfig/libvirt-gconfig-domain-interface-bridge.h> diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index 0a12ce4..8e23b87 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -739,6 +739,15 @@ global: gvir_config_domain_hostdev_get_readonly; gvir_config_domain_hostdev_get_shareable; gvir_config_domain_hostdev_get_type; + gvir_config_domain_hostdev_pci_get_address; + gvir_config_domain_hostdev_pci_get_managed; + gvir_config_domain_hostdev_pci_get_rom; + gvir_config_domain_hostdev_pci_get_type; + gvir_config_domain_hostdev_pci_new; + gvir_config_domain_hostdev_pci_new_from_xml; + gvir_config_domain_hostdev_pci_set_address; + gvir_config_domain_hostdev_pci_set_managed; + gvir_config_domain_hostdev_pci_set_rom; gvir_config_domain_hostdev_set_boot_order; gvir_config_domain_hostdev_set_readonly; gvir_config_domain_hostdev_set_shareable; -- 2.5.0
On Thu, Jan 28, 2016 at 04:32:13PM +0100, Zeeshan Ali (Khattak) wrote:
Add API to read and write PCI hostdev nodes. --- libvirt-gconfig/Makefile.am | 2 + .../libvirt-gconfig-domain-hostdev-pci.c | 211 +++++++++++++++++++++ .../libvirt-gconfig-domain-hostdev-pci.h | 80 ++++++++ libvirt-gconfig/libvirt-gconfig-domain-hostdev.c | 2 +- libvirt-gconfig/libvirt-gconfig.h | 1 + libvirt-gconfig/libvirt-gconfig.sym | 9 + 6 files changed, 304 insertions(+), 1 deletion(-) create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.c create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.h
diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am index 4294bab..245313d 100644 --- a/libvirt-gconfig/Makefile.am +++ b/libvirt-gconfig/Makefile.am @@ -51,6 +51,7 @@ GCONFIG_HEADER_FILES = \ libvirt-gconfig-domain-graphics-spice.h \ libvirt-gconfig-domain-graphics-vnc.h \ libvirt-gconfig-domain-hostdev.h \ + libvirt-gconfig-domain-hostdev-pci.h \ libvirt-gconfig-domain-input.h \ libvirt-gconfig-domain-interface.h \ libvirt-gconfig-domain-interface-bridge.h \ @@ -143,6 +144,7 @@ GCONFIG_SOURCE_FILES = \ libvirt-gconfig-domain-graphics-spice.c \ libvirt-gconfig-domain-graphics-vnc.c \ libvirt-gconfig-domain-hostdev.c \ + libvirt-gconfig-domain-hostdev-pci.c \ libvirt-gconfig-domain-input.c \ libvirt-gconfig-domain-interface.c \ libvirt-gconfig-domain-interface-bridge.c \ diff --git a/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.c b/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.c new file mode 100644 index 0000000..ed1d146 --- /dev/null +++ b/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.c @@ -0,0 +1,211 @@ +/* + * libvirt-gconfig-domain-hostdev.c: libvirt domain hostdev configuration + * + * Copyright (C) 2012 Red Hat, Inc.
Can be 2012-2016 as well.
+ * + * 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@gnome.org> + * Christophe Fergeau <cfergeau@redhat.com> + */ + +#include <config.h> + +#include "libvirt-gconfig/libvirt-gconfig.h" +#include "libvirt-gconfig/libvirt-gconfig-private.h" + +#define GVIR_CONFIG_DOMAIN_HOSTDEV_PCI_GET_PRIVATE(obj) \ + (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, GVirConfigDomainHostdevPciPrivate)) + +struct _GVirConfigDomainHostdevPciPrivate +{ + gboolean unused; +}; + +G_DEFINE_TYPE(GVirConfigDomainHostdevPci, gvir_config_domain_hostdev_pci, GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV); + +static void gvir_config_domain_hostdev_pci_class_init(GVirConfigDomainHostdevPciClass *klass) +{ + g_type_class_add_private(klass, sizeof(GVirConfigDomainHostdevPciPrivate)); +} + + +static void gvir_config_domain_hostdev_pci_init(GVirConfigDomainHostdevPci *hostdev) +{ + hostdev->priv = GVIR_CONFIG_DOMAIN_HOSTDEV_PCI_GET_PRIVATE(hostdev); +} + +/** + * gvir_config_domain_hostdev_pci_new: + * + * Creates a new #GVirConfigDomainHostdevPci with a reference count of 1. + * + * Returns: a new #GVirConfigDomainHostdevPci
It may be me who initially introduced the "with a reference count of 1" wording, but now I find it odd. I'd favour « Returns:(transfer full): a new #GVirConfigDomainHostdevPci. The returned object should be unreffed with g_object_unref() when no longer needed. » which is also used in other places in libvirt-glib.
+ */ +GVirConfigDomainHostdevPci *gvir_config_domain_hostdev_pci_new(void) +{ + GVirConfigObject *object; + + object = gvir_config_object_new(GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, + "hostdev", NULL); + gvir_config_object_set_attribute(object, "mode", "subsystem", NULL); + gvir_config_object_set_attribute(object, "type", "pci", NULL); + + return GVIR_CONFIG_DOMAIN_HOSTDEV_PCI(object); +} + +/** + * gvir_config_domain_hostdev_pci_new_from_xml: + * @xml: xml data to create the host device from + * @error: return location for a #GError, or NULL + * + * Creates a new #GVirConfigDomainHostdevPci with a reference count of 1. + * The host device object will be created using the XML description stored + * in @xml. This is a fragment of libvirt domain XML whose root node is + * <hostdev>. + * + * Returns: a new #GVirConfigDomainHostdevPci, or NULL if @xml failed to + * be parsed. + */ +GVirConfigDomainHostdevPci *gvir_config_domain_hostdev_pci_new_from_xml(const gchar *xml, + GError **error) +{ + GVirConfigObject *object; + + object = gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, + "hostdev", NULL, xml, error); + if (*error != NULL) + return NULL; + + if (g_strcmp0(gvir_config_object_get_attribute(object, NULL, "type"), "pci") != 0) { + g_object_unref(G_OBJECT(object)); + g_return_val_if_reached(NULL); + } + + return GVIR_CONFIG_DOMAIN_HOSTDEV_PCI(object); +} + +void gvir_config_domain_hostdev_pci_set_address(GVirConfigDomainHostdevPci *hostdev, + GVirConfigDomainAddressPci *address) +{ + GVirConfigObject *source; + GVirConfigObject *addr_object; + xmlNodePtr node; + xmlAttrPtr attr; + + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev)); + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_ADDRESS_PCI(address)); + addr_object = GVIR_CONFIG_OBJECT(address); + node = gvir_config_object_get_xml_node(addr_object); + g_return_if_fail(node != NULL); + + source = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(hostdev), + "source"); + /* We can't just use GVirConfigDomainAddressPci's node, as is, since it + * contains a 'type' attribute that's not valid in this context. So we + * create a copy for our use and just delete the 'type' node from it. + */
It took me a while to understand what this comment meant exactly, and why this was needed. If I followed correctly, in libvirt RelaxNG schema, the address for a PCI hostdev device is a 'pciaddress', which do not have a 'type' attribute contrary to most other addresses. This means that for the PCI address of a hostdev device, trying to set a 'type' attribute will trigger errors from libvirt when it tries to parse the domain XML. In my opinion, this is a libvirt bug that type="pci" is not accepted here as libvirt documentation says: « Device Addresses Many devices have an optional <address> sub-element to describe where the device is placed on the virtual bus presented to the guest.[...] Every address has a mandatory attribute type that describes which bus the device is on. » Maybe here things are a bit special as this address is not a direct child of the <hostdev> element, but is contained within a <source> element, but I still think it would be nicer of libvirt, and more consistent to accept an optional type="pci" attribute here rather than rejecting it. This would have spared us the ugly workaround below :(
+ node = xmlCopyNode(node, 1); + for (attr = node->properties; attr; attr = attr->next) { + if (g_strcmp0 ("type", (char *)attr->name) == 0) { + xmlRemoveProp (attr); + break; + } + } + gvir_config_object_set_child(source, node);
It's _probably_ fine (I'm always wary of higher level classes having to directly use libxml rather than going through GvirConfigObject).
+} + +GVirConfigDomainAddressPci *gvir_config_domain_hostdev_pci_get_address(GVirConfigDomainHostdevPci *hostdev) +{ + GVirConfigObject *source; + GVirConfigObject* address; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev), NULL); + + source = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(hostdev), "source"); + if (source == NULL) + return NULL; + + address = gvir_config_object_get_child_with_type(source, + "address", + GVIR_CONFIG_TYPE_DOMAIN_ADDRESS_PCI); + return GVIR_CONFIG_DOMAIN_ADDRESS_PCI(address); +}
Since this address is not really a GVirConfigDomainAddressPci, it's a bit odd to return one anyway... Let's say it's going to be fine this way.
+ +void gvir_config_domain_hostdev_pci_set_managed(GVirConfigDomainHostdevPci *hostdev, + gboolean managed) +{ + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev)); + + gvir_config_object_set_attribute(GVIR_CONFIG_OBJECT(hostdev), + "managed", + managed? "yes": "no", + NULL);
Actually, gvir_config_object_set_attribute_with_type with G_TYPE_BOOLEAN is going to do the right thing here.
+} + +gboolean gvir_config_domain_hostdev_pci_get_managed(GVirConfigDomainHostdevPci *hostdev) +{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev), FALSE); + + return gvir_config_object_get_attribute_boolean(GVIR_CONFIG_OBJECT(hostdev), + NULL, + "managed", + FALSE); +} + +void gvir_config_domain_hostdev_pci_set_rom(GVirConfigDomainHostdevPci *hostdev, + const gchar *rom_file, + gboolean bar) +{ + GVirConfigObject *rom; + xmlNodePtr rom_node; + + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev)); + + rom = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(hostdev), "rom"); + rom_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(rom)); + + xmlSetProp(rom_node, + (const xmlChar *) "bar", + bar? (const xmlChar *) "on" : (const xmlChar *) "off"); + xmlSetProp(rom_node, + (const xmlChar *) "file", + (const xmlChar *) rom_file); +}
gvir_config_object_set_attribute() can be used instead.
+ +const gchar *gvir_config_domain_hostdev_pci_get_rom(GVirConfigDomainHostdevPci *hostdev, + gboolean *bar) +{ + xmlNodePtr hostdev_node; + xmlNodePtr rom_node; + const gchar *bar_str; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev), NULL); + + hostdev_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(hostdev)); + g_return_val_if_fail(hostdev_node != NULL, NULL); + + rom_node = gvir_config_xml_get_element(hostdev_node, "rom", NULL); + if (!rom_node || !(rom_node->children)) + return NULL; + + bar_str = gvir_config_xml_get_attribute_content(rom_node, "bar"); + if (g_strcmp0(bar_str, "on")) + *bar = TRUE; + else + *bar = FALSE; + + return (const char *) rom_node->children->content;
The filename is in the file attribute, it's not in the node content (addressed in a patch I'm going to send by switching to using GVirConfigObject helpers). Regarding the API, I don't think there are other places in libvirt-gconfig where we set (or get) 2 things with a single setter/getter. Are these 2 parameters tightly coupled together? It seems to me we could do something similar to the <os><type> attributes ('arch' and 'machine'). These 2 attributes are set by 2 separate helpers, but these helpers are in the GVirConfigOs class: gvir_config_domain_os_set_arch gvir_config_domain_os_set_machine
+} diff --git a/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.h b/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.h new file mode 100644 index 0000000..e03681e --- /dev/null +++ b/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.h @@ -0,0 +1,80 @@ +/* + * libvirt-gconfig-domain-hostdev.h: libvirt domain hostdev configuration + * + * Copyright (C) 2012 Red Hat, Inc.
2012-2016 Christophe
Hi,
+ */ +GVirConfigDomainHostdevPci *gvir_config_domain_hostdev_pci_new(void) +{ + GVirConfigObject *object; + + object = gvir_config_object_new(GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, + "hostdev", NULL); + gvir_config_object_set_attribute(object, "mode", "subsystem", NULL); + gvir_config_object_set_attribute(object, "type", "pci", NULL); + + return GVIR_CONFIG_DOMAIN_HOSTDEV_PCI(object); +} + +/** + * gvir_config_domain_hostdev_pci_new_from_xml: + * @xml: xml data to create the host device from + * @error: return location for a #GError, or NULL + * + * Creates a new #GVirConfigDomainHostdevPci with a reference count of 1. + * The host device object will be created using the XML description stored + * in @xml. This is a fragment of libvirt domain XML whose root node is + * <hostdev>. + * + * Returns: a new #GVirConfigDomainHostdevPci, or NULL if @xml failed to + * be parsed. + */ +GVirConfigDomainHostdevPci *gvir_config_domain_hostdev_pci_new_from_xml(const gchar *xml, + GError **error) +{ + GVirConfigObject *object; + + object = gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, + "hostdev", NULL, xml, error); + if (*error != NULL) + return NULL; + + if (g_strcmp0(gvir_config_object_get_attribute(object, NULL, "type"), "pci") != 0) { + g_object_unref(G_OBJECT(object)); + g_return_val_if_reached(NULL); + } + + return GVIR_CONFIG_DOMAIN_HOSTDEV_PCI(object); +} + +void gvir_config_domain_hostdev_pci_set_address(GVirConfigDomainHostdevPci *hostdev, + GVirConfigDomainAddressPci *address) +{ + GVirConfigObject *source; + GVirConfigObject *addr_object; + xmlNodePtr node; + xmlAttrPtr attr; + + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev)); + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_ADDRESS_PCI(address)); + addr_object = GVIR_CONFIG_OBJECT(address); + node = gvir_config_object_get_xml_node(addr_object); + g_return_if_fail(node != NULL); + + source = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(hostdev), + "source"); + /* We can't just use GVirConfigDomainAddressPci's node, as is, since it + * contains a 'type' attribute that's not valid in this context. So we + * create a copy for our use and just delete the 'type' node from it. + */
It took me a while to understand what this comment meant exactly, and why this was needed. If I followed correctly, in libvirt RelaxNG schema, the address for a PCI hostdev device is a 'pciaddress', which do not have a 'type' attribute contrary to most other addresses. This means that for the PCI address of a hostdev device, trying to set a 'type' attribute will trigger errors from libvirt when it tries to parse the domain XML.
Yeah, I tried tried with `virsh edit` and it tells me xml doesn't confirm to schema.
In my opinion, this is a libvirt bug that type="pci" is not accepted here as libvirt documentation says: « Device Addresses
Many devices have an optional <address> sub-element to describe where the device is placed on the virtual bus presented to the guest.[...]
Every address has a mandatory attribute type that describes which bus the device is on. »
Maybe here things are a bit special as this address is not a direct child of the <hostdev> element, but is contained within a <source> element, but I still think it would be nicer of libvirt, and more consistent to accept an optional type="pci" attribute here rather than rejecting it. This would have spared us the ugly workaround below :(
Yeah but even if it's resolved in libvirt, we'd still want to have a work around for older libvirt.
+ +const gchar *gvir_config_domain_hostdev_pci_get_rom(GVirConfigDomainHostdevPci *hostdev, + gboolean *bar) +{ + xmlNodePtr hostdev_node; + xmlNodePtr rom_node; + const gchar *bar_str; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev), NULL); + + hostdev_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(hostdev)); + g_return_val_if_fail(hostdev_node != NULL, NULL); + + rom_node = gvir_config_xml_get_element(hostdev_node, "rom", NULL); + if (!rom_node || !(rom_node->children)) + return NULL; + + bar_str = gvir_config_xml_get_attribute_content(rom_node, "bar"); + if (g_strcmp0(bar_str, "on")) + *bar = TRUE; + else + *bar = FALSE; + + return (const char *) rom_node->children->content;
The filename is in the file attribute, it's not in the node content (addressed in a patch I'm going to send by switching to using GVirConfigObject helpers).
Regarding the API, I don't think there are other places in libvirt-gconfig where we set (or get) 2 things with a single setter/getter. Are these 2 parameters tightly coupled together? It seems to me we could do something similar to the <os><type> attributes ('arch' and 'machine'). These 2 attributes are set by 2 separate helpers, but these helpers are in the GVirConfigOs class: gvir_config_domain_os_set_arch gvir_config_domain_os_set_machine
Both 'arch' and 'machine' are separate attributes on the 'type' node but "bar" is an attribute of "rom" node, that I think is unlikely to be used in isolation. If we keep this API, I think I should change 'rom' to be nullable. I see your point though and I don't have hard feeling either way. -- Regards, Zeeshan Ali (Khattak) ________________________________________ Befriend GNOME: http://www.gnome.org/friends/
On Mon, Feb 08, 2016 at 04:58:34PM +0000, Zeeshan Ali (Khattak) wrote:
Hi,
+ */ +GVirConfigDomainHostdevPci *gvir_config_domain_hostdev_pci_new(void) +{ + GVirConfigObject *object; + + object = gvir_config_object_new(GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, + "hostdev", NULL); + gvir_config_object_set_attribute(object, "mode", "subsystem", NULL); + gvir_config_object_set_attribute(object, "type", "pci", NULL); + + return GVIR_CONFIG_DOMAIN_HOSTDEV_PCI(object); +} + +/** + * gvir_config_domain_hostdev_pci_new_from_xml: + * @xml: xml data to create the host device from + * @error: return location for a #GError, or NULL + * + * Creates a new #GVirConfigDomainHostdevPci with a reference count of 1. + * The host device object will be created using the XML description stored + * in @xml. This is a fragment of libvirt domain XML whose root node is + * <hostdev>. + * + * Returns: a new #GVirConfigDomainHostdevPci, or NULL if @xml failed to + * be parsed. + */ +GVirConfigDomainHostdevPci *gvir_config_domain_hostdev_pci_new_from_xml(const gchar *xml, + GError **error) +{ + GVirConfigObject *object; + + object = gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, + "hostdev", NULL, xml, error); + if (*error != NULL) + return NULL; + + if (g_strcmp0(gvir_config_object_get_attribute(object, NULL, "type"), "pci") != 0) { + g_object_unref(G_OBJECT(object)); + g_return_val_if_reached(NULL); + } + + return GVIR_CONFIG_DOMAIN_HOSTDEV_PCI(object); +} + +void gvir_config_domain_hostdev_pci_set_address(GVirConfigDomainHostdevPci *hostdev, + GVirConfigDomainAddressPci *address) +{ + GVirConfigObject *source; + GVirConfigObject *addr_object; + xmlNodePtr node; + xmlAttrPtr attr; + + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev)); + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_ADDRESS_PCI(address)); + addr_object = GVIR_CONFIG_OBJECT(address); + node = gvir_config_object_get_xml_node(addr_object); + g_return_if_fail(node != NULL); + + source = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(hostdev), + "source"); + /* We can't just use GVirConfigDomainAddressPci's node, as is, since it + * contains a 'type' attribute that's not valid in this context. So we + * create a copy for our use and just delete the 'type' node from it. + */
It took me a while to understand what this comment meant exactly, and why this was needed. If I followed correctly, in libvirt RelaxNG schema, the address for a PCI hostdev device is a 'pciaddress', which do not have a 'type' attribute contrary to most other addresses. This means that for the PCI address of a hostdev device, trying to set a 'type' attribute will trigger errors from libvirt when it tries to parse the domain XML.
Yeah, I tried tried with `virsh edit` and it tells me xml doesn't confirm to schema.
What I mainly meant was that it would be nice to improve this comment, ie replace "in this context" with something like "for addresses used in a hostdevice node context".
In my opinion, this is a libvirt bug that type="pci" is not accepted here as libvirt documentation says: « Device Addresses
Many devices have an optional <address> sub-element to describe where the device is placed on the virtual bus presented to the guest.[...]
Every address has a mandatory attribute type that describes which bus the device is on. »
Maybe here things are a bit special as this address is not a direct child of the <hostdev> element, but is contained within a <source> element, but I still think it would be nicer of libvirt, and more consistent to accept an optional type="pci" attribute here rather than rejecting it. This would have spared us the ugly workaround below :(
Yeah but even if it's resolved in libvirt, we'd still want to have a work around for older libvirt.
Yes, of course, all I'm saying is that this looks like we are working around a libvirt bug (either code/rng or documentation). In both case we should make sure it's fixed/known, even if we'll have to deal with it in libvirt-glib anyway.
+ +const gchar *gvir_config_domain_hostdev_pci_get_rom(GVirConfigDomainHostdevPci *hostdev, + gboolean *bar) +{ + xmlNodePtr hostdev_node; + xmlNodePtr rom_node; + const gchar *bar_str; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev), NULL); + + hostdev_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(hostdev)); + g_return_val_if_fail(hostdev_node != NULL, NULL); + + rom_node = gvir_config_xml_get_element(hostdev_node, "rom", NULL); + if (!rom_node || !(rom_node->children)) + return NULL; + + bar_str = gvir_config_xml_get_attribute_content(rom_node, "bar"); + if (g_strcmp0(bar_str, "on")) + *bar = TRUE; + else + *bar = FALSE; + + return (const char *) rom_node->children->content;
The filename is in the file attribute, it's not in the node content (addressed in a patch I'm going to send by switching to using GVirConfigObject helpers).
Regarding the API, I don't think there are other places in libvirt-gconfig where we set (or get) 2 things with a single setter/getter. Are these 2 parameters tightly coupled together? It seems to me we could do something similar to the <os><type> attributes ('arch' and 'machine'). These 2 attributes are set by 2 separate helpers, but these helpers are in the GVirConfigOs class: gvir_config_domain_os_set_arch gvir_config_domain_os_set_machine
Both 'arch' and 'machine' are separate attributes on the 'type' node but "bar" is an attribute of "rom" node, that I think is unlikely to be used in isolation. If we keep this API, I think I should change 'rom' to be nullable. I see your point though and I don't have hard feeling either way.
This _get_rom() method is returning a const char * and a gboolean bar, I assume the returned const char * is the 'file' attribute (iirc that's consistent with what is done in the unit test), and the gboolean is the bar. So we are also talking about 2 separate attributes on the "rom" node here. Christophe
--- tests/test-gconfig.c | 30 +++++++++++++++++++++++++ tests/xml/gconfig-domain-device-pci-hostdev.xml | 11 +++++++++ 2 files changed, 41 insertions(+) create mode 100644 tests/xml/gconfig-domain-device-pci-hostdev.xml diff --git a/tests/test-gconfig.c b/tests/test-gconfig.c index be55ef9..c3711f1 100644 --- a/tests/test-gconfig.c +++ b/tests/test-gconfig.c @@ -709,6 +709,34 @@ static void test_domain_device_usb_redir(void) g_object_unref(G_OBJECT(domain)); } +static void test_domain_device_pci_hostdev(void) +{ + GVirConfigDomain *domain; + GVirConfigDomainAddressPci *address; + GVirConfigDomainHostdevPci *hostdev; + + domain = gvir_config_domain_new(); + + hostdev = gvir_config_domain_hostdev_pci_new(); + gvir_config_domain_hostdev_set_boot_order(GVIR_CONFIG_DOMAIN_HOSTDEV(hostdev), 1); + gvir_config_domain_hostdev_pci_set_managed(hostdev, TRUE); + gvir_config_domain_hostdev_pci_set_rom(hostdev, "/etc/fake/boot.bin", TRUE); + + address = gvir_config_domain_address_pci_new(); + gvir_config_domain_address_pci_set_domain(address, 1); + gvir_config_domain_address_pci_set_bus(address, 2); + gvir_config_domain_address_pci_set_slot(address, 3); + gvir_config_domain_address_pci_set_function(address, 4); + gvir_config_domain_hostdev_pci_set_address(hostdev, address); + g_object_unref(G_OBJECT(address)); + + gvir_config_domain_add_device(domain, GVIR_CONFIG_DOMAIN_DEVICE (hostdev)); + g_object_unref(G_OBJECT(hostdev)); + + check_xml(domain, "gconfig-domain-device-pci-hostdev.xml"); + + g_object_unref(G_OBJECT(domain)); +} int main(int argc, char **argv) { @@ -739,6 +767,8 @@ int main(int argc, char **argv) test_domain_device_channel); g_test_add_func("/libvirt-gconfig/domain-device-usb-redir", test_domain_device_usb_redir); + g_test_add_func("/libvirt-gconfig/domain-device-pci-hostdev", + test_domain_device_pci_hostdev); return g_test_run(); } diff --git a/tests/xml/gconfig-domain-device-pci-hostdev.xml b/tests/xml/gconfig-domain-device-pci-hostdev.xml new file mode 100644 index 0000000..70e32ac --- /dev/null +++ b/tests/xml/gconfig-domain-device-pci-hostdev.xml @@ -0,0 +1,11 @@ +<domain> + <devices> + <hostdev mode="subsystem" type="pci" managed="yes"> + <boot order="1"/> + <rom bar="on" file="/etc/fake/boot.bin"/> + <source> + <address domain="0x0001" bus="0x02" slot="0x03" function="0x4"/> + </source> + </hostdev> + </devices> +</domain> -- 2.5.0
ACK, though I have some additions to this test to exercise the getters as well. Christophe On Thu, Jan 28, 2016 at 04:32:14PM +0100, Zeeshan Ali (Khattak) wrote:
--- tests/test-gconfig.c | 30 +++++++++++++++++++++++++ tests/xml/gconfig-domain-device-pci-hostdev.xml | 11 +++++++++ 2 files changed, 41 insertions(+) create mode 100644 tests/xml/gconfig-domain-device-pci-hostdev.xml
diff --git a/tests/test-gconfig.c b/tests/test-gconfig.c index be55ef9..c3711f1 100644 --- a/tests/test-gconfig.c +++ b/tests/test-gconfig.c @@ -709,6 +709,34 @@ static void test_domain_device_usb_redir(void) g_object_unref(G_OBJECT(domain)); }
+static void test_domain_device_pci_hostdev(void) +{ + GVirConfigDomain *domain; + GVirConfigDomainAddressPci *address; + GVirConfigDomainHostdevPci *hostdev; + + domain = gvir_config_domain_new(); + + hostdev = gvir_config_domain_hostdev_pci_new(); + gvir_config_domain_hostdev_set_boot_order(GVIR_CONFIG_DOMAIN_HOSTDEV(hostdev), 1); + gvir_config_domain_hostdev_pci_set_managed(hostdev, TRUE); + gvir_config_domain_hostdev_pci_set_rom(hostdev, "/etc/fake/boot.bin", TRUE); + + address = gvir_config_domain_address_pci_new(); + gvir_config_domain_address_pci_set_domain(address, 1); + gvir_config_domain_address_pci_set_bus(address, 2); + gvir_config_domain_address_pci_set_slot(address, 3); + gvir_config_domain_address_pci_set_function(address, 4); + gvir_config_domain_hostdev_pci_set_address(hostdev, address); + g_object_unref(G_OBJECT(address)); + + gvir_config_domain_add_device(domain, GVIR_CONFIG_DOMAIN_DEVICE (hostdev)); + g_object_unref(G_OBJECT(hostdev)); + + check_xml(domain, "gconfig-domain-device-pci-hostdev.xml"); + + g_object_unref(G_OBJECT(domain)); +}
int main(int argc, char **argv) { @@ -739,6 +767,8 @@ int main(int argc, char **argv) test_domain_device_channel); g_test_add_func("/libvirt-gconfig/domain-device-usb-redir", test_domain_device_usb_redir); + g_test_add_func("/libvirt-gconfig/domain-device-pci-hostdev", + test_domain_device_pci_hostdev);
return g_test_run(); } diff --git a/tests/xml/gconfig-domain-device-pci-hostdev.xml b/tests/xml/gconfig-domain-device-pci-hostdev.xml new file mode 100644 index 0000000..70e32ac --- /dev/null +++ b/tests/xml/gconfig-domain-device-pci-hostdev.xml @@ -0,0 +1,11 @@ +<domain> + <devices> + <hostdev mode="subsystem" type="pci" managed="yes"> + <boot order="1"/> + <rom bar="on" file="/etc/fake/boot.bin"/> + <source> + <address domain="0x0001" bus="0x02" slot="0x03" function="0x4"/> + </source> + </hostdev> + </devices> +</domain> -- 2.5.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@gnome.org> + * Christophe Fergeau <cfergeau@redhat.com> + */ + +#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?
+ +void gvir_config_domain_hostdev_set_shareable(GVirConfigDomainHostdev *hostdev, + gboolean shareable) +{ + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV(hostdev)); + + if (shareable) { + GVirConfigObject *node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(hostdev), "shareable");
I'd split this one too.
+ g_object_unref(node); + } else { + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(hostdev), "shareable", NULL); + } +} + +gboolean gvir_config_domain_hostdev_get_shareable(GVirConfigDomainHostdev *hostdev) +{ + xmlNodePtr hostdev_node; + xmlNodePtr shareable_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); + + shareable_node = gvir_config_xml_get_element(hostdev_node, "shareable", NULL); + + return (shareable_node != NULL); +}
_get_readonly and _get_shareable are mostly the same code, I'd add a gvir_config_object_has_child() helper to group that common code. Can you also add API doc for these new methods? Looks good otherwise, Christophe
Hi, On Fri, Jan 29, 2016 at 5:18 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Hey,
A gconfig: prefix in the commit shortlog would be nice
Hmm.. maybe for consistency, yes but I'll remove "GVirConfig" then from class name.
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@gnome.org> + * Christophe Fergeau <cfergeau@redhat.com> + */ + +#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?
Well it says it's currently only supported by them, not that it's only applicable to them so i kept it in the base class but maybe that was not a good call. -- Regards, Zeeshan Ali (Khattak) ________________________________________ Befriend GNOME: http://www.gnome.org/friends/
On Mon, Feb 08, 2016 at 04:22:47PM +0000, Zeeshan Ali (Khattak) wrote:
Hi,
On Fri, Jan 29, 2016 at 5:18 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Hey,
A gconfig: prefix in the commit shortlog would be nice
Hmm.. maybe for consistency, yes but I'll remove "GVirConfig" then from class name.
Please keep both :)
+ 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?
Well it says it's currently only supported by them, not that it's only applicable to them so i kept it in the base class but maybe that was not a good call.
Hmm could probably make sense with pci in the future, and the wording can indeed be interpreted both ways. Let's keep things this way. Christophe
participants (2)
-
Christophe Fergeau -
Zeeshan Ali (Khattak)