Re: [PATCH 2/5] parallels: dump info about container filesystems

Michal Privoznik <mprivozn@xxxxxxxxxx> · Mon, 12 Jan 2015 16:39:22 +0100

On 26.12.2014 15:43, Dmitry Guryanov wrote:
Obtain information about container's filesystems and
store it in virDomainDef structure.

Signed-off-by: Dmitry Guryanov <dguryanov@xxxxxxxxxxxxx>
---
  src/parallels/parallels_sdk.c | 63 ++++++++++++++++++++++++++++++++++++++++---
  1 file changed, 60 insertions(+), 3 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 0b5430d..5f494f2 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -535,6 +535,55 @@ prlsdkGetDiskInfo(PRL_HANDLE prldisk,
  }

  static int
+prlsdkGetFSInfo(PRL_HANDLE prldisk,
+                virDomainFSDefPtr fs)
+{
+    char *buf = NULL;
+    PRL_UINT32 buflen = 0;
+    PRL_RESULT pret;
+    int ret = -1;
+
+    fs->type = VIR_DOMAIN_FS_TYPE_FILE;
+    fs->fsdriver = VIR_DOMAIN_FS_DRIVER_TYPE_PLOOP;
+    fs->accessmode = VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH;
+    fs->wrpolicy = VIR_DOMAIN_FS_WRPOLICY_DEFAULT;
+    fs->format = VIR_STORAGE_FILE_PLOOP;
+
+    fs->readonly = false;
+    fs->symlinksResolved = false;
+
+    pret = PrlVmDev_GetImagePath(prldisk, NULL, &buflen);
+    prlsdkCheckRetGoto(pret, cleanup);
+
+    if (VIR_ALLOC_N(buf, buflen) < 0)
+        goto cleanup;
+
+    pret = PrlVmDev_GetImagePath(prldisk, buf, &buflen);
+    prlsdkCheckRetGoto(pret, cleanup);
+
+    fs->src = buf;
+    buf = NULL;
+
+    pret = PrlVmDevHd_GetMountPoint(prldisk, NULL, &buflen);
+    prlsdkCheckRetGoto(pret, cleanup);
+
+    if (VIR_ALLOC_N(buf, buflen) < 0)
+        goto cleanup;
+
+    pret = PrlVmDevHd_GetMountPoint(prldisk, buf, &buflen);
+    prlsdkCheckRetGoto(pret, cleanup);
+
+    fs->dst = buf;
+    buf = NULL;
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(buf);
+    return ret;
+}
+
+static int
  prlsdkAddDomainHardDisksInfo(PRL_HANDLE sdkdom, virDomainDefPtr def)
  {
      PRL_RESULT pret;
@@ -542,6 +591,7 @@ prlsdkAddDomainHardDisksInfo(PRL_HANDLE sdkdom, virDomainDefPtr def)
      PRL_UINT32 i;
      PRL_HANDLE hdd = PRL_INVALID_HANDLE;
      virDomainDiskDefPtr disk = NULL;
+    virDomainFSDefPtr fs;

This needs to be initialized to NULL, otherwise you may end up freeing a 
random memory if the control jumps to 'error' label.


      pret = PrlVmCfg_GetHardDisksCount(sdkdom, &hddCount);
      prlsdkCheckRetGoto(pret, error);
@@ -551,9 +601,15 @@ prlsdkAddDomainHardDisksInfo(PRL_HANDLE sdkdom, virDomainDefPtr def)
          prlsdkCheckRetGoto(pret, error);

          if (IS_CT(def)) {
-            /* TODO: convert info about disks in container
-             * to virDomainFSDef structs */
-            VIR_WARN("Skipping disk information for container");
+
+            if (VIR_ALLOC(fs) < 0)
+                goto error;
+
+            if (prlsdkGetFSInfo(hdd, fs) < 0)
+                goto error;
+
+            if (virDomainFSInsert(def, fs) < 0)
+                goto error;

What are you missing here is fs = NULL; after the insert. Otherwise in 
next iteration - if we fail - we jump to 'error' and free @fs which, 
however, is already part of domain @def. Same applies for @disk. 
Previously it wasn't problem as there was only @disk which would get 
overwritten as the first thing in new loop.

              PrlHandle_Free(hdd);
              hdd = PRL_INVALID_HANDLE;
@@ -577,6 +633,7 @@ prlsdkAddDomainHardDisksInfo(PRL_HANDLE sdkdom, virDomainDefPtr def)
   error:
      PrlHandle_Free(hdd);
      virDomainDiskDefFree(disk);
+    virDomainFSDefFree(fs);
      return -1;
  }



Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list






