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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list