[libvirt] [PATCH 4/5] vz: report disks either as disks or filesystems depending on original xml
Maxim Nestratov
mnestratov at virtuozzo.com
Wed Dec 21 16:57:27 UTC 2016
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 <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.
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;
>>
>>
More information about the libvir-list
mailing list