Re: [PATCH 4/4] vz: support filesystem type volume

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06.07.2016 17:02, Olga Krishtal wrote:
> Vz containers are able to use ploop volumes from storage pools
> to work upon.
> 
> To use filesystem type volume, pool name and volume name should be
> specifaed in <source> :
>    <filesystem type='volume' accessmode='passthrough'>
>       <driver type='ploop' format='ploop'/>
>       <source pool='guest_images' volume='TEST_POOL_CT'/>
>       <target dir='/'/>
>    </filesystem>
> 
> The information about pool and volume is stored in ct dom configuration:
> <StorageURL>libvirt://localhost/pool_name/vol_name</StorageURL>
> and can be easily obtained via PrlVmDevHd_GetStorageURL sdk call.
> 
> The only shorcoming: if storage pool is moved somewhere the ct
> should be redefined in order to refresh the information aboot path
> to root.hdd
> 
> Signed-off-by: Olga Krishtal <okrishtal@xxxxxxxxxxxxx>
> ---
>  src/storage/storage_driver.c |   4 +-
>  src/vz/vz_driver.c           |   2 +-
>  src/vz/vz_sdk.c              | 126 +++++++++++++++++++++++++++++++++++++++----
>  src/vz/vz_sdk.h              |   2 +-
>  4 files changed, 120 insertions(+), 14 deletions(-)
> 
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index cb9d578..99e9df2 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -3508,7 +3508,9 @@ virStorageTranslateDiskSourcePool(virConnectPtr conn,
>          case VIR_STORAGE_VOL_BLOCK:
>              def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK;
>              break;
> -
> +        case VIR_STORAGE_VOL_PLOOP:
> +            def->src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE;
> +            break;

please keep original spacing, keep emply line

>          case VIR_STORAGE_VOL_NETWORK:
>          case VIR_STORAGE_VOL_NETDIR:
>              virReportError(VIR_ERR_INTERNAL_ERROR,
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index b17cea7..811a3c4 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -762,7 +762,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
>              if (prlsdkCreateVm(driver, def))
>                  goto cleanup;
>          } else if (def->os.type == VIR_DOMAIN_OSTYPE_EXE) {
> -            if (prlsdkCreateCt(driver, def))
> +            if (prlsdkCreateCt(conn, driver, def))

driver is the top most object, convention is to place it object with broader scope first
here driver is better place before conn

>                  goto cleanup;
>          } else {
>              virReportError(VIR_ERR_INVALID_ARG,
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index dd7eb6e..43832f0 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -33,6 +33,7 @@
>  #include "virtime.h"
>  #include "virhostcpu.h"
>  
> +#include "storage/storage_driver.h"
>  #include "vz_sdk.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_PARALLELS
> @@ -645,6 +646,9 @@ prlsdkGetFSInfo(PRL_HANDLE prldisk,
>  {
>      char *buf = NULL;
>      int ret = -1;
> +    char *storage = NULL;

i'd use existing buf

> +    char **matches = NULL;
> +    virURIPtr uri = NULL;
>  
>      fs->type = VIR_DOMAIN_FS_TYPE_FILE;
>      fs->fsdriver = VIR_DOMAIN_FS_DRIVER_TYPE_PLOOP;
> @@ -655,12 +659,50 @@ prlsdkGetFSInfo(PRL_HANDLE prldisk,
>      fs->readonly = false;
>      fs->symlinksResolved = false;
>  
> -    if (!(buf = prlsdkGetStringParamVar(PrlVmDev_GetImagePath, prldisk)))
> +    if (!(storage = prlsdkGetStringParamVar(PrlVmDevHd_GetStorageURL, prldisk)))
>          goto cleanup;
>  
> -    fs->src->path = buf;
> -    buf = NULL;
> +    if (!virStringIsEmpty(storage)) {
> +        if (!(uri = virURIParse(storage)))
> +            goto cleanup;
> +        if (STRNEQ("libvirt", uri->scheme)) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("unknown backend for filesystem: '%s'"),
> +                           uri->scheme);

i'd better type 'unexpected uri scheme', backend is not relevant in this context.

> +            goto cleanup;
> +        }
> +
> +        if (!(matches = virStringSplitCount(uri->path, "/", 0, NULL)) ||
> +            !matches[0]) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("splitting StorageUrl failed %s"), uri->path);
> +            goto cleanup;
> +        }
> +        if (!matches[1]) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("can't identify pool %s"), matches[1]);

message is not very useful, matches[1] is NULL ))

> +            goto cleanup;
> +        }
> +        if (!matches[2]) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("can't identify volume %s"), matches[2]);
> +            goto cleanup;
> +        }

i'd merge above checks into one using a chain of ||.

> +        fs->type = VIR_DOMAIN_FS_TYPE_VOLUME;
> +        if (VIR_ALLOC(fs->src->srcpool) < 0)
> +            goto cleanup;
> +        if (VIR_STRDUP(fs->src->srcpool->pool, matches[1]) < 0)
> +            goto cleanup;
> +        if (VIR_STRDUP(fs->src->srcpool->volume, matches[2]) < 0)
> +            goto cleanup;
> +    } else {

i would move "fs->type = VIR_DOMAIN_FS_TYPE_FILE" here

> +
> +        if (!(buf = prlsdkGetStringParamVar(PrlVmDev_GetImagePath, prldisk)))
> +            goto cleanup;
>  
> +        fs->src->path = buf;
> +        buf = NULL;
> +    }
>      if (!(buf = prlsdkGetStringParamVar(PrlVmDevHd_GetMountPoint, prldisk)))
>          goto cleanup;
>  
> @@ -671,6 +713,8 @@ prlsdkGetFSInfo(PRL_HANDLE prldisk,
>  
>   cleanup:
>      VIR_FREE(buf);
> +    VIR_FREE(storage);
> +    virStringFreeList(matches);
>      return ret;
>  }
>  
> @@ -1680,7 +1724,6 @@ prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom)
>      PRL_HANDLE job;
>  
>      virCheckNonNullArgGoto(dom, error);
> -

irrelevant

>      pdom = dom->privateData;
>      sdkdom = prlsdkSdkDomainLookupByUUID(driver, dom->def->uuid);
>      if (sdkdom == PRL_INVALID_HANDLE)
> @@ -1721,7 +1764,6 @@ prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom)
>      /* depends on prlsdkAddVNCInfo */
>      if (prlsdkAddDomainHardware(driver, sdkdom, def) < 0)
>          goto error;
> -

irrelevant

>      /* depends on prlsdkAddDomainHardware */
>      if (prlsdkConvertBootOrder(sdkdom, def) < 0)
>          goto error;
> @@ -2695,7 +2737,8 @@ static int prlsdkCheckNetUnsupportedParams(virDomainNetDefPtr net)
>  
>  static int prlsdkCheckFSUnsupportedParams(virDomainFSDefPtr fs)
>  {
> -    if (fs->type != VIR_DOMAIN_FS_TYPE_FILE) {
> +    if (fs->type != VIR_DOMAIN_FS_TYPE_FILE &&
> +        fs->type != VIR_DOMAIN_FS_TYPE_VOLUME) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                         _("Only file based filesystems are "
>                           "supported by vz driver."));

i think message need to be updated

> @@ -3508,12 +3551,14 @@ prlsdkUpdateDevice(vzDriverPtr driver,
>      return 0;
>  }
>  
> +
>  static int
>  prlsdkAddFS(PRL_HANDLE sdkdom, virDomainFSDefPtr fs)
>  {
>      PRL_RESULT pret;
>      PRL_HANDLE sdkdisk = PRL_INVALID_HANDLE;
>      int ret = -1;
> +    char *storage = NULL;
>  
>      if (fs->type == VIR_DOMAIN_FS_TYPE_TEMPLATE)
>          return 0;
> @@ -3524,6 +3569,14 @@ prlsdkAddFS(PRL_HANDLE sdkdom, virDomainFSDefPtr fs)
>      pret = PrlVmCfg_CreateVmDev(sdkdom, PDE_HARD_DISK, &sdkdisk);
>      prlsdkCheckRetGoto(pret, cleanup);
>  
> +    if (fs->type == VIR_DOMAIN_FS_TYPE_VOLUME) {
> +        if (virAsprintf(&storage, "libvirt://localhost/%s/%s", fs->src->srcpool->pool,
> +                        fs->src->srcpool->volume) < 0)
> +            goto cleanup;
> +        pret = PrlVmDevHd_SetStorageURL(sdkdisk, storage);
> +        prlsdkCheckRetGoto(pret, cleanup);
> +    }
> +
>      pret = PrlVmDev_SetEnabled(sdkdisk, 1);
>      prlsdkCheckRetGoto(pret, cleanup);
>  
> @@ -3548,6 +3601,7 @@ prlsdkAddFS(PRL_HANDLE sdkdom, virDomainFSDefPtr fs)
>      ret = 0;
>  
>   cleanup:
> +    VIR_FREE(storage);
>      PrlHandle_Free(sdkdisk);
>      return ret;
>  }
> @@ -3767,7 +3821,6 @@ prlsdkDoApplyConfig(vzDriverPtr driver,
>          if (prlsdkSetBootOrderVm(sdkdom, def) < 0)
>              goto error;
>      }
> -
>      return 0;
>  
>   error:
> @@ -3791,7 +3844,6 @@ prlsdkApplyConfig(vzDriverPtr driver,
>      sdkdom = prlsdkSdkDomainLookupByUUID(driver, dom->def->uuid);
>      if (sdkdom == PRL_INVALID_HANDLE)
>          return -1;
> -
>      job = PrlVm_BeginEdit(sdkdom);
>      if (PRL_FAILED(waitJob(job)))
>          return -1;
> @@ -3803,7 +3855,6 @@ prlsdkApplyConfig(vzDriverPtr driver,
>          if (PRL_FAILED(waitJob(job)))
>              ret = -1;
>      }
> -
>      PrlHandle_Free(sdkdom);
>  
>      return ret;
> @@ -3848,8 +3899,52 @@ prlsdkCreateVm(vzDriverPtr driver, virDomainDefPtr def)
>      return ret;
>  }
>  
> +static int
> +virStorageTranslatePoolLocal(virConnectPtr conn, virStorageSourcePtr src)
> +{
> +
> +    virStoragePoolPtr pool = NULL;
> +    virStorageVolPtr vol = NULL;
> +    virStorageVolInfo info;
> +    int ret = -1;
> +
> +    if (!(pool = virStoragePoolLookupByName(conn, src->srcpool->pool)))
> +        return -1;
> +    if (virStoragePoolIsActive(pool) != 1) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("storage pool '%s' containing volume '%s' "
> +                         "is not active"), src->srcpool->pool,
> +                       src->srcpool->volume);
> +        goto cleanup;
> +    }
> +
> +    if (!(vol = virStorageVolLookupByName(pool, src->srcpool->volume)))
> +        goto cleanup;
> +
> +    if (virStorageVolGetInfo(vol, &info) < 0)
> +        goto cleanup;
> +
> +    if (info.type != VIR_STORAGE_VOL_PLOOP) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Unsupported volume format '%s'"),
> +                       virStorageVolTypeToString(info.type));
> +        goto cleanup;
> +    }
> +
> +    if (!(src->path = virStorageVolGetPath(vol)))
> +        goto cleanup;
> +
> +    ret = 0;
> +
> + cleanup:
> +    virObjectUnref(pool);
> +    virObjectUnref(vol);
> +    return ret;
> +}
> +
> +
>  int
> -prlsdkCreateCt(vzDriverPtr driver, virDomainDefPtr def)
> +prlsdkCreateCt(virConnectPtr conn, vzDriverPtr driver, virDomainDefPtr def)
>  {
>      PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
>      PRL_GET_VM_CONFIG_PARAM_DATA confParam;
> @@ -3859,9 +3954,18 @@ prlsdkCreateCt(vzDriverPtr driver, virDomainDefPtr def)
>      PRL_UINT32 flags;
>      int ret = -1;
>      int useTemplate = 0;
> +    size_t i;
>  
> -    if (def->nfss == 1 && def->fss[0]->type == VIR_DOMAIN_FS_TYPE_TEMPLATE)
> +    if (def->nfss == 1 && def->fss[0]->type == VIR_DOMAIN_FS_TYPE_TEMPLATE) {
>          useTemplate = 1;
> +    } else if (def->fss[0]->type != VIR_DOMAIN_FS_TYPE_FILE) {

why you check def->fss[0]->type ?

> +        for (i = 0; i < def->nfss; i++) {
> +            if (def->fss[i]->type == VIR_DOMAIN_FS_TYPE_VOLUME) {
> +                if (virStorageTranslatePoolLocal(conn, def->fss[i]->src) < 0)
> +                    goto cleanup;
> +            }
> +        }
> +    }
>  
>      confParam.nVmType = PVT_CT;
>      confParam.sConfigSample = "vswap.1024MB";
> diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
> index 41557a3..4767fdd 100644
> --- a/src/vz/vz_sdk.h
> +++ b/src/vz/vz_sdk.h
> @@ -57,7 +57,7 @@ prlsdkApplyConfig(vzDriverPtr driver,
>                    virDomainObjPtr dom,
>                    virDomainDefPtr new);
>  int prlsdkCreateVm(vzDriverPtr driver, virDomainDefPtr def);
> -int prlsdkCreateCt(vzDriverPtr driver, virDomainDefPtr def);
> +int prlsdkCreateCt(virConnectPtr conn, vzDriverPtr driver, virDomainDefPtr def);
>  int
>  prlsdkUnregisterDomain(vzDriverPtr driver, virDomainObjPtr dom, unsigned int flags);
>  int
> 

please drop irrelevant spacing changes, I've marked a few of them

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]