[libvirt] [PATCH 3/3] vz: support boot order in domain xml dump

[Nikolay Shirokovskiy]

On 07.04.2016 12:54, Maxim Nestratov wrote:
> 22.03.2016 16:56, Nikolay Shirokovskiy пишет:
>> As usual we try to deal correctly with vz domains that were
>> created by other means and thus can have all range of SDK domain
>> parameters. If vz domain boot order can't be represented
>> in libvirt os boot section let's give warning and make os boot section
>> represent SDK to some extent.
>>
>> 1. Os boot section supports up to 4 boot devices. Here we just
>> cut SDK boot order up to this limit. Not too bad.
>>
>> 2. If there is a floppy in boot order let's just skip it.
>> Anyway we don't show it in the xml. Not too bad too.
>>
>> 3. SDK boot order with unsupported disks order. Say we have "hdb, hda" in
>> SDK. We can not present this thru os boot order. Well let's just
>> give warning but leave double <boot dev='hd'/> in xml. It's
>> kind of misleading but we warn you!
>>
>> SDK boot order have an extra parameters 'inUse' and 'sequenceIndex'
>> which makes our task more complicated. In realitly however 'inUse'
>> is always on and 'sequenceIndex == boot position index + 1' which
>> simplifies out task back again! To be on a safe side let's explicitly
>> check for this conditions!
>>
>> We have another exercise here. We want to check for unrepresentable
>> condition 3 (see above). The tricky part is that in contrast to
>> domains defined thru this driver 3-rd party defined domains can
>> have device ordering different from default. Thus we need
>> some id to check that N-th boot disk of os boot section is same as
>> N-th boot disk of SDK boot. This is what prlsdkBootOrderCheck
>> for. It uses disks sources paths as id for disks and iface names
>> for network devices.
>>
>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
>> ---
>>   src/vz/vz_sdk.c | 238 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 238 insertions(+)
>>
>> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
>> index f059a8e..6cecb93 100644
>> --- a/src/vz/vz_sdk.c
>> +++ b/src/vz/vz_sdk.c

..

>> +static int
>> +prlsdkConvertBootOrder(PRL_HANDLE sdkdom, virDomainDefPtr def)
>> +{
>> +    int ret = -1;
>> +    PRL_RESULT pret;
>> +    PRL_UINT32 bootNum;
>> +    PRL_HANDLE bootDev = PRL_INVALID_HANDLE;
>> +    PRL_BOOL inUse;
>> +    PRL_DEVICE_TYPE sdkType;
>> +    virDomainBootOrder type;
>> +    PRL_UINT32 bootIndex, sdkIndex;
>> +    int bootUsage[VIR_DOMAIN_BOOT_LAST] = { 0 };
>> +    size_t i;
>> +
>> +    pret = PrlVmCfg_GetBootDevCount(sdkdom, &bootNum);
>> +    prlsdkCheckRetExit(pret, -1);
>> +
>> +    def->os.nBootDevs = 0;
>> +
>> +    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) {
>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                           _("Boot ordering with disabled items is not supported"));
>> +            goto cleanup;
>> +        }
>> +
>> +        pret = PrlBootDev_GetSequenceIndex(bootDev, &bootIndex);
>> +        prlsdkCheckRetGoto(pret, cleanup);
>> +
>> +        /* bootIndex is started from 1 */
>> +        if (bootIndex != i + 1) {
>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                           _("Unsupported boot order configuration"));
>> +            goto cleanup;
>> +        }
>> +
> 
> This check doesn't work because boot indexes are not necessarily continuous. The only condition we should check here is that they should be growing.
> 
> I added the following chunk to your code and if you don't mind I can squash it to your patch and push.
> 
> @@ -1432,7 +1432,7 @@ prlsdkConvertBootOrder(PRL_HANDLE sdkdom, virDomainDefPtr def)
>      PRL_BOOL inUse;
>      PRL_DEVICE_TYPE sdkType;
>      virDomainBootOrder type;
> -    PRL_UINT32 bootIndex, sdkIndex;
> +    PRL_UINT32 prevBootIndex = 0, bootIndex, sdkIndex;
>      int bootUsage[VIR_DOMAIN_BOOT_LAST] = { 0 };
>      size_t i;
> 
> @@ -1463,11 +1463,12 @@ prlsdkConvertBootOrder(PRL_HANDLE sdkdom, virDomainDefPtr def)
>          prlsdkCheckRetGoto(pret, cleanup);
> 
>          /* bootIndex is started from 1 */
> -        if (bootIndex != i + 1) {
> +        if (bootIndex <= prevBootIndex) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                             _("Unsupported boot order configuration"));
>              goto cleanup;
>          }
> +        prevBootIndex = bootIndex;
> 
>          pret = PrlBootDev_GetType(bootDev, &sdkType);
>          prlsdkCheckRetGoto(pret, cleanup);
> 

ok, but don't forget to update commit message and comment too