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

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

 




On 08.04.2016 19:39, 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>
> 
> Signed-off-by: Olga Krishtal <okrishtal@xxxxxxxxxxxxx>
> ---
>  src/storage/storage_driver.c |   3 +
>  src/vz/vz_sdk.c              | 127 ++++++++++++++++++++++++++++++++++---------
>  2 files changed, 105 insertions(+), 25 deletions(-)
> 
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 1d96618..c2c1483 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -3348,6 +3348,9 @@ virStorageTranslateDiskSourcePool(virConnectPtr conn,
>              def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK;
>              break;
>  
> +        case VIR_STORAGE_VOL_PLOOP:
> +            def->src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE;
> +
>          case VIR_STORAGE_VOL_NETWORK:
>          case VIR_STORAGE_VOL_NETDIR:
>              virReportError(VIR_ERR_INTERNAL_ERROR,
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 00f42b8..a8b2ffa 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -31,6 +31,8 @@
>  #include "datatypes.h"
>  #include "domain_conf.h"
>  #include "virtime.h"
> +#include "dirname.h"

seems we don't use it

> +#include "storage/storage_driver.h"
>  
>  #include "vz_sdk.h"
>  
> @@ -570,8 +572,36 @@ prlsdkGetFSInfo(PRL_HANDLE prldisk,
>      PRL_UINT32 buflen = 0;
>      PRL_RESULT pret;
>      int ret = -1;
> +    char *storage = NULL;
> +    char **matches = NULL;
> +    virURIPtr uri = NULL;
> +
> +    pret = PrlVmDevHd_GetStorageURL(prldisk, NULL, &buflen);
> +    prlsdkCheckRetGoto(pret, cleanup);
> +    if (VIR_ALLOC_N(storage, buflen) < 0)
> +        goto cleanup;
> +    pret = PrlVmDevHd_GetStorageURL(prldisk, storage, &buflen);
> +    prlsdkCheckRetGoto(pret, cleanup);

there is brand new prlsdkGetStringParamVar for getting strings from sdk

> +
> +    if (!virStringIsEmpty(storage)) {
> +        uri = virURIParse(storage);
> +        if (!uri || STRNEQ("volume", uri->scheme))
> +            goto cleanup;

second clause need error message

> +        if (!(matches = virStringSplitCount(uri->path, "/", 0, NULL)))
> +            goto cleanup;

check count or we have invalid pointers in matches[N]
or add matches[0] to the check.

> +        if (!matches[1] || !matches[2])
> +            goto cleanup;
> +        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 {
> +        fs->type = VIR_DOMAIN_FS_TYPE_FILE;

i think we should move setting fs->src->path under this branch, we
don't need this set in case of volume AFAIU

> +    }
>  
> -    fs->type = VIR_DOMAIN_FS_TYPE_FILE;
>      fs->fsdriver = VIR_DOMAIN_FS_DRIVER_TYPE_PLOOP;
>      fs->accessmode = VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH;
>      fs->wrpolicy = VIR_DOMAIN_FS_WRPOLICY_DEFAULT;
> @@ -608,6 +638,9 @@ prlsdkGetFSInfo(PRL_HANDLE prldisk,
>  
>   cleanup:
>      VIR_FREE(buf);
> +    VIR_FREE(storage);
> +    virURIFree(uri);
> +    virStringFreeList(matches);
>      return ret;
>  }
>  
> @@ -636,7 +669,7 @@ prlsdkAddDomainHardDisksInfo(vzConnPtr privconn, PRL_HANDLE sdkdom, virDomainDef
>  
>          if (PDT_USE_REAL_DEVICE != emulatedType && IS_CT(def)) {
>  
> -            if (VIR_ALLOC(fs) < 0)
> +            if (!(fs = virDomainFSDefNew()))
>                  goto error;
>  
>              if (prlsdkGetFSInfo(hdd, fs) < 0)
> @@ -2417,13 +2450,14 @@ 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."));
> +                       _("Unsupported filesystem type."));
>          return -1;
>      }
>  
> +

unrelated, stricly speaking )

>      if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_PLOOP) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                         _("Only ploop fs driver is "
> @@ -3266,6 +3300,7 @@ prlsdkAddFS(PRL_HANDLE sdkdom, virDomainFSDefPtr fs)
>      PRL_RESULT pret;
>      PRL_HANDLE sdkdisk = PRL_INVALID_HANDLE;
>      int ret = -1;
> +    char *storage = NULL;
>  
>      if (prlsdkCheckFSUnsupportedParams(fs) < 0)
>          return -1;
> @@ -3273,6 +3308,13 @@ 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, "volume///%s/%s", fs->src->srcpool->pool,

volume:

> +                        fs->src->srcpool->volume) < 0)
> +            goto cleanup;
> +        pret = PrlVmDevHd_SetStorageURL(sdkdisk, storage);
> +        prlsdkCheckRetGoto(pret, cleanup);
> +    }
>      pret = PrlVmDev_SetEnabled(sdkdisk, 1);
>      prlsdkCheckRetGoto(pret, cleanup);
>  
> @@ -3297,6 +3339,7 @@ prlsdkAddFS(PRL_HANDLE sdkdom, virDomainFSDefPtr fs)
>      ret = 0;
>  
>   cleanup:
> +    VIR_FREE(storage);
>      PrlHandle_Free(sdkdisk);
>      return ret;
>  }
> @@ -3388,10 +3431,10 @@ prlsdkDoApplyConfig(virConnectPtr conn,
>      }
>  
>      for (i = 0; i < def->nfss; i++) {
> -        if (STREQ(def->fss[i]->dst, "/"))
> -            needBoot = false;
> -        if (prlsdkAddFS(sdkdom, def->fss[i]) < 0)
> -            goto error;
> +            if (STREQ(def->fss[i]->dst, "/"))
> +                needBoot = false;
> +            if (prlsdkAddFS(sdkdom, def->fss[i]) < 0)
> +                goto error;

unrelated

>      }
>  
>      for (i = 0; i < def->ndisks; i++) {
> @@ -3493,6 +3536,49 @@ prlsdkCreateVm(virConnectPtr conn, 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(virConnectPtr conn, virDomainDefPtr def)
>  {
> @@ -3506,25 +3592,16 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def)
>      int useTemplate = 0;
>      size_t i;
>  
> -    if (def->nfss > 1) {
> -        /* Check all filesystems */
> -        for (i = 0; i < def->nfss; i++) {
> -            if (def->fss[i]->type != VIR_DOMAIN_FS_TYPE_FILE) {
> -                virReportError(VIR_ERR_INVALID_ARG, "%s",
> -                               _("Unsupported filesystem type."));
> -                return -1;
> -            }
> -        }
> -    } else if (def->nfss == 1) {
> -        if (def->fss[0]->type == VIR_DOMAIN_FS_TYPE_TEMPLATE) {
> +    if (def->nfss == 1 && def->fss[0]->type == VIR_DOMAIN_FS_TYPE_TEMPLATE)
>              useTemplate = 1;

indentation

> -        } else if (def->fss[0]->type != VIR_DOMAIN_FS_TYPE_FILE) {
> -            virReportError(VIR_ERR_INVALID_ARG, "%s",
> -                           _("Unsupported filesystem type."));
> -            return -1;

you take benefits of prlsdkCheckFSUnsupportedParams checks and drop
/ * Check all filesystems */ above. This is not obvious and is not
this patch intention. Please move to a distinct patch.

> +
> +    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;

we don't need to tranlate at this point, please move to prlsdkAddFS

>          }
> -    }
>  
> +    }
>      confParam.nVmType = PVT_CT;
>      confParam.sConfigSample = "vswap.1024MB";
>      confParam.nOsVersion = 0;
> 

--
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]