[virt-tools-list] [PATCH libvirt-gconfig] Fix generation of filesystem device source XML element

[Daniel P. Berrange]

On Mon, Jun 18, 2012 at 10:21:01AM +0200, Christophe Fergeau wrote:
> On Fri, Jun 15, 2012 at 03:57:21PM +0100, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> > 
> > When setting the filesystem source type, the code forgot to
> > update priv->type. Thus when setting the source element,
> > the incorrect attribute was being used.
> > ---
> >  .../libvirt-gconfig-domain-device-private.h        |    3 +++
> >  libvirt-gconfig/libvirt-gconfig-domain-device.c    |    2 +-
> >  libvirt-gconfig/libvirt-gconfig-domain-filesys.c   |   28 ++++++++++++++++++++
> >  3 files changed, 32 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libvirt-gconfig/libvirt-gconfig-domain-device-private.h b/libvirt-gconfig/libvirt-gconfig-domain-device-private.h
> > index f50946a..3ec83c3 100644
> > --- a/libvirt-gconfig/libvirt-gconfig-domain-device-private.h
> > +++ b/libvirt-gconfig/libvirt-gconfig-domain-device-private.h
> > @@ -37,6 +37,9 @@ GVirConfigDomainDevice *
> >  gvir_config_domain_disk_new_from_tree(GVirConfigXmlDoc *doc,
> >                                        xmlNodePtr tree);
> >  GVirConfigDomainDevice *
> > +gvir_config_domain_filesys_new_from_tree(GVirConfigXmlDoc *doc,
> > +                                         xmlNodePtr tree);
> > +GVirConfigDomainDevice *
> >  gvir_config_domain_graphics_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 9a7fae5..60d6bcc 100644
> > --- a/libvirt-gconfig/libvirt-gconfig-domain-device.c
> > +++ b/libvirt-gconfig/libvirt-gconfig-domain-device.c
> > @@ -62,7 +62,7 @@ gvir_config_domain_device_new_from_tree(GVirConfigXmlDoc *doc,
> >      if (xmlStrEqual(tree->name, (xmlChar*)"disk")) {
> >          return gvir_config_domain_disk_new_from_tree(doc, tree);
> >      } else if (xmlStrEqual(tree->name, (xmlChar*)"filesystem")) {
> > -        type = GVIR_CONFIG_TYPE_DOMAIN_FILESYS;
> > +        return gvir_config_domain_filesys_new_from_tree(doc, tree);
> >      } else if (xmlStrEqual(tree->name, (xmlChar*)"controller")) {
> >          return gvir_config_domain_controller_new_from_tree(doc, tree);
> >      } else if (xmlStrEqual(tree->name, (xmlChar*)"lease")) {
> > diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
> > index 904a7a3..cc2f604 100644
> > --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
> > +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
> > @@ -69,6 +69,33 @@ GVirConfigDomainFilesys *gvir_config_domain_filesys_new_from_xml(const gchar *xm
> >      return GVIR_CONFIG_DOMAIN_FILESYS(object);
> >  }
> >  
> > +GVirConfigDomainDevice *
> > +gvir_config_domain_filesys_new_from_tree(GVirConfigXmlDoc *doc,
> > +                                      xmlNodePtr tree)
> > +{
> > +    GVirConfigObject *object;
> > +    GVirConfigDomainFilesys *filesys;
> > +    GVirConfigDomainFilesysType type;
> > +    const char *type_str;
> > +
> > +    type_str = gvir_config_xml_get_attribute_content(tree, "type");
> > +    if (type_str == NULL)
> > +        return NULL;
> > +
> > +    type = gvir_config_genum_get_value(GVIR_CONFIG_TYPE_DOMAIN_FILESYS_TYPE,
> > +                                       type_str,
> > +                                       GVIR_CONFIG_DOMAIN_FILESYS_FILE);
> > +    if (type == -1)
> > +        return NULL;
> 
> This cannot happen, gvir_config_genum_get_value will always return the
> default value (GVIR_CONFIG_DOMAIN_FILESYS_FILE) if there's a failure.
> Apart from this issue, it looks good.

Hehe, you better tell that to yourself then - I copy+pasted this code
from the libvirt-gconfig-domain-disk.c code which you wrote ;-P

Seriously though, I'll post a patch to fix existing instances of this
flaw.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|