[libvirt] [PATCH 3/3] parallels: Set the first HDD from XML as bootable

[Michal Privoznik]

On 17.02.2015 12:52, Mikhail Feoktistov wrote:
> 1. Delete all boot devices for VM instance
> 2. Find the first HDD from XML and set it as bootable
> 
> Now we support only one boot device and it should be HDD
> ---
>  src/parallels/parallels_sdk.c |   95 +++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 91 insertions(+), 4 deletions(-)
> 
> diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
> index 8660259..15822ab 100644
> --- a/src/parallels/parallels_sdk.c
> +++ b/src/parallels/parallels_sdk.c
> @@ -1987,6 +1987,69 @@ static int prlsdkClearDevices(PRL_HANDLE sdkdom)
>      return ret;
>  }
>  
> +int

static int. This function is used only within the parallels_sdk.c file.

> +prlsdkRemoveBootDevices(PRL_HANDLE sdkdom)
> +{
> +    PRL_RESULT pret;
> +    PRL_UINT32 devCount;
> +    PRL_HANDLE dev = PRL_INVALID_HANDLE;
> +    PRL_DEVICE_TYPE devType;
> +
> +    pret = PrlVmCfg_GetBootDevCount(sdkdom, &devCount);
> +    prlsdkCheckRetGoto(pret, error);
> +
> +    for (int i = 0; i < devCount; i++)	{

We don't use c99 style of for loops. Moreover, i should be the same type
as devCount.

> +
> +        /* always get device by index 0, because device list resort after delete */
> +        pret = PrlVmCfg_GetBootDev(sdkdom, 0, &dev);
> +        prlsdkCheckRetGoto(pret, error);
> +
> +        pret = PrlBootDev_GetType(dev, &devType);
> +        prlsdkCheckRetGoto(pret, error);
> + 

Trailing whitespace.

> +        pret = PrlBootDev_Remove(dev);
> +        prlsdkCheckRetGoto(pret, error);
> +    }
> +
> +    return 0;
> +
> + error:
> +    return -1;
> +}
> +
> +int

Again. static int.

> +prlsdkAddDeviceToBootList(PRL_HANDLE sdkdom,
> +                          PRL_UINT32 devIndex,
> +                          PRL_DEVICE_TYPE devType,
> +                          PRL_UINT32 bootSequence)
> +{
> +    PRL_RESULT pret;
> +    PRL_HANDLE bootDev = PRL_INVALID_HANDLE;
> +
> +    pret = PrlVmCfg_CreateBootDev(sdkdom, &bootDev);
> +    prlsdkCheckRetGoto(pret, error);
> +
> +    pret = PrlBootDev_SetIndex(bootDev, devIndex);
> +    prlsdkCheckRetGoto(pret, error);
> +
> +    pret = PrlBootDev_SetType(bootDev, devType);
> +    prlsdkCheckRetGoto(pret, error);
> +
> +    pret = PrlBootDev_SetSequenceIndex(bootDev, bootSequence);
> +    prlsdkCheckRetGoto(pret, error);
> +
> +    pret = PrlBootDev_SetInUse(bootDev, PRL_TRUE);
> +    prlsdkCheckRetGoto(pret, error);
> +    

Trailing whitepsace.

> +    return 0;
> +
> + error:
> +    if (bootDev != PRL_INVALID_HANDLE)
> +        PrlBootDev_Remove(bootDev);
> +
> +    return -1;
> +}
> +
>  static int prlsdkCheckGraphicsUnsupportedParams(virDomainDefPtr def)
>  {
>      virDomainGraphicsDefPtr gr;
> @@ -2601,7 +2664,7 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, virDomainNetDefPtr net)
>      return ret;
>  }
>  
> -static int prlsdkAddDisk(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk)
> +static int prlsdkAddDisk(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk, bool bootDisk)
>  {
>      PRL_RESULT pret;
>      PRL_HANDLE sdkdisk = PRL_INVALID_HANDLE;
> @@ -2610,14 +2673,18 @@ static int prlsdkAddDisk(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk)
>      PRL_MASS_STORAGE_INTERFACE_TYPE sdkbus;
>      int idx;
>      virDomainDeviceDriveAddressPtr drive;
> +    PRL_UINT32 devIndex;
> +    PRL_DEVICE_TYPE devType;
>  
>      if (prlsdkCheckDiskUnsupportedParams(disk) < 0)
>          return -1;
>  
>      if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)
> -        pret = PrlVmCfg_CreateVmDev(sdkdom, PDE_HARD_DISK, &sdkdisk);
> +        devType = PDE_HARD_DISK;
>      else
> -        pret = PrlVmCfg_CreateVmDev(sdkdom, PDE_OPTICAL_DISK, &sdkdisk);
> +        devType = PDE_OPTICAL_DISK;
> +
> +    pret = PrlVmCfg_CreateVmDev(sdkdom, devType, &sdkdisk);
>      prlsdkCheckRetGoto(pret, cleanup);
>  
>      pret = PrlVmDev_SetEnabled(sdkdisk, 1);
> @@ -2713,6 +2780,14 @@ static int prlsdkAddDisk(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk)
>          goto cleanup;
>      }
>  
> +    if (bootDisk == true) {
> +        pret = PrlVmDev_GetIndex(sdkdisk, &devIndex);
> +        prlsdkCheckRetGoto(pret, cleanup);
> +
> +        if (prlsdkAddDeviceToBootList(sdkdom, devIndex, devType, 0) < 0)
> +            goto cleanup;
> +    }
> +
>      return 0;
>   cleanup:
>      PrlHandle_Free(sdkdisk);
> @@ -2796,6 +2871,9 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom,
>      if (prlsdkClearDevices(sdkdom) < 0)
>          goto error;
>  
> +    if (prlsdkRemoveBootDevices(sdkdom) < 0)
> +        goto error;
> +
>      if (prlsdkApplyGraphicsParams(sdkdom, def) < 0)
>          goto error;
>  
> @@ -2812,8 +2890,17 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom,
>              goto error;
>      }
>  
> +    bool needBoot = true;

This has to go up in the beginning of the function, otherwise the
compiler complains that variable initialization is jumped over (all the
gotos above).

>      for (i = 0; i < def->ndisks; i++) {
> -        if (prlsdkAddDisk(sdkdom, def->disks[i]) < 0)
> +        bool bootDisk = false;
> +
> +        if (needBoot == true &&
> +            def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
> +
> +            needBoot = false;
> +            bootDisk = true;
> +        }
> +        if (prlsdkAddDisk(sdkdom, def->disks[i], bootDisk) < 0)
>              goto error;
>      }
>  
> 

I've fixed all the nits, ACKed and pushed all three patches.
Congratulations on your first libvirt contribution!

Michal