[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Daniel P. Berrange schreef:
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.

Ok.


@@ -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.

Ok. Appended xmlFree's.
http://kinkrsoftware.nl/contrib/libvirt/pool.patch



+                        }
+                    }
+                }
             } 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,

It is not leaking imho, since 'down under' there is an xmlFree(source).

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

How can we know if the volume is a file or a device?

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

Mmm... that sounds not trivial... (I mean duplicate wise).



Stefan


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]