[libvirt] Domain XML format using defined storage volume + RFC

[Daniel P. Berrange]

On Fri, May 16, 2008 at 01:00:16AM +0200, Stefan de Konink wrote:
> On Thu, 15 May 2008, Stefan de Konink wrote:
> 
> > On Thu, 15 May 2008, Stefan de Konink wrote:
> >
> > > I almost started crying why it didn't work. But it is fixed, and it works
> > > as a charm :) See updated patch!
> >
> > I guess I need to write the 'check' tool and documentation update too?
> 
> Since there is currently no check build for pools in that directory, I'm
> passing this to someother person.

Yes, testing the interaction with pools is actually harder that I anticipated,
because the libvirtd isn't available in context of the test suite. I'll have to
think about best way todo it. Might be able to stub out a dummy impl for purposes
of testing.

> @@ -1309,11 +1311,24 @@ virDomainParseXMLDiskDesc(virConnectPtr conn, xmlNodePtr node,
>          if (cur->type == XML_ELEMENT_NODE) {
>              if ((source == NULL) &&
>                  (xmlStrEqual(cur->name, BAD_CAST "source"))) {
> -
>                  if (typ == 0)
>                      source = xmlGetProp(cur, BAD_CAST "file");
> -                else
> +                else if (typ == 1)
>                      source = xmlGetProp(cur, BAD_CAST "dev");
> +                else if (typ == 2) {
> +                    xmlChar *pool   = xmlGetProp(cur, BAD_CAST "pool");
> +                    xmlChar *volume = xmlGetProp(cur, BAD_CAST "volume");
> +                    if (pool != NULL && volume != NULL) {
> +                        virStoragePoolPtr virPool;
> +                        virPool = virStoragePoolLookupByName(conn, (const char *) pool);
> +                        if (virPool != NULL) {
> +                            virStorageVolPtr virVol;
> +                            virVol = virStorageVolLookupByName(virPool, (const char *) volume);
> +                            if (virVol != NULL)
> +                                source = BAD_CAST virStorageVolGetPath(virVol);

This is leaking the pool and volume objects.

> +                        }
> +                    }
> +                }
>              } else if ((target == NULL) &&
>                         (xmlStrEqual(cur->name, BAD_CAST "target"))) {
>                  target = xmlGetProp(cur, BAD_CAST "dev");
> @@ -1411,7 +1426,8 @@ virDomainParseXMLDiskDesc(virConnectPtr conn, xmlNodePtr node,
>                  virBufferVSprintf(buf, "(uname 'phy:%s')", source);
>              else
>                  virBufferVSprintf(buf, "(uname 'phy:/dev/%s')", source);
> -        }
> +        } else if (typ == 2)
> +            virBufferVSprintf(buf, "(uname 'phy:%s')", source);

This is leaking the 'source' string, and a volume can be either a file
or a physical device, so fixing it to be 'phy:' is not correct.

We also need to apply the reverse mapping when generating the XML. eg
do a virStorageVolLookupByPath() to discover the volume/pool. 

And really need todo the same for the QEMU driver

Regards,
Daniel.
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|