Re: [libvirt] Domain XML format using defined storage volume + RFC
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 :|