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