subject:"\[libvirt\] \[libvirt\-glib 2\/3\] Add GVirConfigDomainHostdevPci"

Re: [libvirt] [libvirt-glib 2/3] Add GVirConfigDomainHostdevPci

2016-02-09 Thread Christophe Fergeau

On Mon, Feb 08, 2016 at 04:58:34PM +, 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
> >> + * .
> >> + *
> >> + * 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  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  element, but is contained within a 
> > 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)
> >> +{
> >> +

Re: [libvirt] [libvirt-glib 2/3] Add GVirConfigDomainHostdevPci

2016-02-08 Thread Zeeshan Ali (Khattak)

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
>> + * .
>> + *
>> + * 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  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  element, but is contained within a 
> 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;
>

Re: [libvirt] [libvirt-glib 2/3] Add GVirConfigDomainHostdevPci

2016-02-01 Thread Christophe Fergeau

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 000..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
> + * .
> + *
> + * Authors: Zeeshan Ali (Khattak) 
> + *  Christophe Fergeau 
> + */
> +
> +#include 
> +
> +#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(obj

[libvirt] [libvirt-glib 2/3] Add GVirConfigDomainHostdevPci

2016-01-28 Thread Zeeshan Ali (Khattak)

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 000..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
+ * .
+ *
+ * Authors: Zeeshan Ali (Khattak) 
+ *  Christophe Fergeau 
+ */
+
+#include 
+
+#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
+ * .
+ *
+ * Returns: a new #GVirConfigDomainHostdevPci, or NULL if @xml failed to
+ * be parsed.
+ */
+GVirConfigDomainHostdevPci *gvir_config_domain_hostdev_pci_new_from_xml(const 
gchar *xml,
+

Re: [libvirt] [libvirt-glib 2/3] Add GVirConfigDomainHostdevPci

Re: [libvirt] [libvirt-glib 2/3] Add GVirConfigDomainHostdevPci

Re: [libvirt] [libvirt-glib 2/3] Add GVirConfigDomainHostdevPci

[libvirt] [libvirt-glib 2/3] Add GVirConfigDomainHostdevPci

4 matches

Site Navigation

Mail list logo

Footer information