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@xxxxxxxxxxxxx>
> ---
> 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;
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
