[libvirt] [PATCH 4/5] vz: report disks either as disks or filesystems depending on original xml

[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 <mnestratov at 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.

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

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

> +
> +        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) {'

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

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

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