21-Dec-16 10:25, Nikolay Shirokovskiy пишет:


On 09.12.2016 17:36, Maxim Nestratov wrote:
Virtuozzo SDK interface doesn't differ filesystems from disks and sees them as 
disks.
Before, we always mistakenly presented disks based on files as filesystems, 
which is
not completely correct. Now we are going to show either disks or filesystems 
depending
on a hint, which uses boot device section of VZ config. Though this information
doesn't change booting order of a CT, it is used by vz libvirt interface as a 
hint
for libvirt representation of disks. Since now, if we have filesystems in input 
xml,
then we add them to VZ booting devices list and rely on this information to show
corresponding libvirt xml.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/vz/vz_sdk.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++------
  1 file changed, 86 insertions(+), 10 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 1030a1b..9b8b48e 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -50,6 +50,9 @@ static PRL_HANDLE
  prlsdkFindNetByMAC(PRL_HANDLE sdkdom, virMacAddrPtr mac);
  static PRL_HANDLE
  prlsdkGetDisk(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk);
+static bool
+prlsdkInBootList(PRL_HANDLE sdkdom,
+                 PRL_HANDLE sdktargetdev);
/*
   * Log error description
@@ -754,7 +757,8 @@ prlsdkAddDomainHardDisksInfo(vzDriverPtr driver, PRL_HANDLE 
sdkdom, virDomainDef
          pret = PrlVmDev_GetEmulatedType(hdd, &emulatedType);
          prlsdkCheckRetGoto(pret, error);
- if (PDT_USE_REAL_DEVICE != emulatedType && IS_CT(def)) {
+        if (IS_CT(def) &&
+            prlsdkInBootList(sdkdom, hdd)) {
if (!(fs = virDomainFSDefNew()))
                  goto error;
@@ -1551,6 +1555,75 @@ virFindDiskBootIndex(virDomainDefPtr def, 
virDomainDiskDevice type, int index)
      return NULL;
  }
+static bool
+prlsdkInBootList(PRL_HANDLE sdkdom,
+                 PRL_HANDLE sdktargetdev)
+{
+    bool ret = false;
+    PRL_RESULT pret;
+    PRL_UINT32 bootNum;
+    PRL_HANDLE bootDev = PRL_INVALID_HANDLE;
+    PRL_BOOL inUse;
+    PRL_DEVICE_TYPE sdkType;
+    PRL_UINT32 sdkIndex, bootIndex;
+    PRL_UINT32 pos, targetpos;
+    PRL_HANDLE dev = PRL_INVALID_HANDLE;
+    size_t i;
+
+    pret = PrlVmDev_GetStackIndex(sdktargetdev, &targetpos);
+    prlsdkCheckRetExit(pret, -1);
+
+    pret = PrlVmCfg_GetBootDevCount(sdkdom, &bootNum);
+    prlsdkCheckRetExit(pret, -1);
+
+    if (bootNum > VIR_DOMAIN_MAX_BOOT_DEVS) {
+        bootNum = VIR_DOMAIN_MAX_BOOT_DEVS;
+        VIR_WARN("Too many boot devices");
+    }
This is not nesessary. We have VIR_DOMAIN_MAX_BOOT_DEVS limit
only if we want to present boot order in domain definition.

Ok. Seems to be true.

+
+    for (i = 0; i < bootNum; ++i) {
+        pret = PrlVmCfg_GetBootDev(sdkdom, i, &bootDev);
+        prlsdkCheckRetGoto(pret, cleanup);
+
+        pret = PrlBootDev_IsInUse(bootDev, &inUse);
+        prlsdkCheckRetGoto(pret, cleanup);
+
+        if (!inUse) {
+            PrlHandle_Free(bootDev);
+            bootDev = PRL_INVALID_HANDLE;
+            continue;
+        }
+
+        pret = PrlBootDev_GetSequenceIndex(bootDev, &bootIndex);
+        prlsdkCheckRetGoto(pret, cleanup);
bootIndex is not used

Yeah, my first intention was to return it, then I reconsidered it but left the 
variable.


+
+        pret = PrlBootDev_GetType(bootDev, &sdkType);
+        prlsdkCheckRetGoto(pret, cleanup);
+
+        pret = PrlBootDev_GetIndex(bootDev, &sdkIndex);
+        prlsdkCheckRetGoto(pret, cleanup);
+
+        dev = prlsdkGetDevByDevIndex(sdkdom, sdkType, sdkIndex);
+        if (dev == PRL_INVALID_HANDLE)
+            goto cleanup;
+
+        pret = PrlVmDev_GetStackIndex(dev, &pos);
+        prlsdkCheckRetExit(pret, -1);
I would use PrlVmDev_GetType and PrlVmDev_GetIndex for given device
and compare obtained values against sdkType and sdkIndex. It is more straight 
forward.

Ok. I'll try to do it this way.


+
+        PrlHandle_Free(bootDev);
+        bootDev = PRL_INVALID_HANDLE;
+
+        if (pos == targetpos) {
+            ret = true;
+            break;
+        }
+    }
+
+ cleanup:
+    PrlHandle_Free(dev);
+    PrlHandle_Free(bootDev);
+    return ret;
+}
  static int
  prlsdkBootOrderCheck(PRL_HANDLE sdkdom, PRL_DEVICE_TYPE sdkType, int sdkIndex,
                       virDomainDefPtr def, int bootIndex)
@@ -3741,23 +3814,26 @@ prlsdkSetBootOrderCt(PRL_HANDLE sdkdom, virDomainDefPtr 
def)
      size_t i;
      PRL_HANDLE hdd = PRL_INVALID_HANDLE;
      PRL_RESULT pret;
+    bool rootfs = false;
      int ret = -1;
/* if we have root mounted we don't need to explicitly set boot order */
let's move this comment to 'if (!rootfs) {'

Ok

      for (i = 0; i < def->nfss; i++) {
+
+        pret = prlsdkAddDeviceToBootList(sdkdom, i, PDE_HARD_DISK, i + 1);
+        prlsdkCheckRetExit(pret, -1);
As this relies on the fact that we add disks for filesystems first and
then disks for disks let's add comment to prlsdkDoApplyConfig that this
order is now significant.

Makes sense


+
          if (STREQ(def->fss[i]->dst, "/"))
-            return 0;
+            rootfs = true;
      }
- /* else set first hard disk as boot device */
-    pret = prlsdkAddDeviceToBootList(sdkdom, 0, PDE_HARD_DISK, 0);
-    prlsdkCheckRetExit(pret, -1);
-
-    pret = PrlVmCfg_GetHardDisk(sdkdom, 0, &hdd);
-    prlsdkCheckRetExit(pret, -1);
+    if (!rootfs) {
+        pret = PrlVmCfg_GetHardDisk(sdkdom, 0, &hdd);
+        prlsdkCheckRetExit(pret, -1);
If we mix filesystems and disks then first harddisk will represent filesystem.
So we should use def->nfss instead of 0. It is a matter of a distinct patch
I guess, nevertheless let's fix this place too.

Nikolay

Agree. See in the next version


- PrlVmDevHd_SetMountPoint(hdd, "/");
-    prlsdkCheckRetGoto(pret, cleanup);
+        PrlVmDevHd_SetMountPoint(hdd, "/");
+        prlsdkCheckRetGoto(pret, cleanup);
+    }
ret = 0;

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

Reply via email to