On 09.12.2016 17:36, Maxim Nestratov wrote: So this is special case only for containers and only for special 'undefined' value of bus type (we don't set/report bus type if create containers with help of virtuozzo tools). I would code the condition exactly as it is. This patch can turn into scsi unexpected cases. Nikolay

Before, boot devices information for CTs was always empty and we didn't indicate that containers always boot from disk. Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com&gt; --- src/vz/vz_sdk.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 9976e4c..a6236d0 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1634,7 +1634,7 @@ prlsdkBootOrderCheck(PRL_HANDLE sdkdom, PRL_DEVICE_TYPE sdkType, int sdkIndex, } static int -prlsdkConvertBootOrder(PRL_HANDLE sdkdom, virDomainDefPtr def) +prlsdkConvertBootOrderVm(PRL_HANDLE sdkdom, virDomainDefPtr def) { int ret = -1; PRL_RESULT pret; @@ -1792,9 +1792,14 @@ prlsdkLoadDomain(vzDriverPtr driver, if (prlsdkAddDomainHardware(driver, sdkdom, def) < 0) goto error; - /* depends on prlsdkAddDomainHardware */ - if (prlsdkConvertBootOrder(sdkdom, def) < 0) - goto error; + if (IS_CT(def)) { + def->os.nBootDevs = 1; + def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK; + } else { + /* depends on prlsdkAddDomainHardware */ + if (prlsdkConvertBootOrderVm(sdkdom, def) < 0) + goto error; + } pret = PrlVmCfg_GetEnvId(sdkdom, &envId); prlsdkCheckRetGoto(pret, error); -- 2.4.11

20-Dec-16 13:50, Nikolay Shirokovskiy пишет: Ok. Makes sense to move this change after #4 and rely on def->fss previously constructed

On 09.12.2016 17:36, Maxim Nestratov wrote: I would drop last sentence. prlsdkApplyVideoParams shows that we ignore video params for containers. Let's reword commit message as we discussed offline - virDomainDefAddImplicitDevices is just never intended for containers. ACK

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(a)virtuozzo.com&gt; --- 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"); + } + + 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); + + 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); + + 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 */ for (i = 0; i < def->nfss; i++) { + + pret = prlsdkAddDeviceToBootList(sdkdom, i, PDE_HARD_DISK, i + 1); + prlsdkCheckRetExit(pret, -1); + 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); - PrlVmDevHd_SetMountPoint(hdd, "/"); - prlsdkCheckRetGoto(pret, cleanup); + PrlVmDevHd_SetMountPoint(hdd, "/"); + prlsdkCheckRetGoto(pret, cleanup); + } ret = 0; -- 2.4.11

21-Dec-16 10:25, Nikolay Shirokovskiy пишет: Ok. Seems to be true. Yeah, my first intention was to return it, then I reconsidered it but left the variable. Ok. I'll try to do it this way. Ok Makes sense Agree. See in the next version

On 09.12.2016 17:36, Maxim Nestratov wrote: I would calculate prefix for containers outside the scope of this switch. Even if it means bigger diff, the switch deserves its own function anyway. Otherwise ACK.