On 23/06/16 16:56, Nikolay Shirokovskiy wrote:
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@virtuozzo.com> --- 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
Ok. I see that there is a lot of changes in prlsdkGetFSInfo. Why prlsdkSetFSInfo is left out of scope?
+ + if (!virStringIsEmpty(storage)) { + uri = virURIParse(storage); + if (!uri || STRNEQ("volume", uri->scheme)) + goto cleanup; second clause need error message
Ok
+ 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.
Will check it out.
+ 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 No, At first change of structure in fs, and than new type
+ }
- 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 ) Related, strictly speaking - if I have type VIR_DOMAIN_FS_TYPE_VOLUME and there is no such change - i will get : "Only file based filesystems are supported by vz driver" or whatever
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: Why "volume:", we have decided and discussed it, isn't it?
+ 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 agree
}
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;
- } 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
indentation Ok this patch intention. Please move to a distinct patch. I do need check with VIR_DOMAIN_FS_TYPE_VOLUME, so I leave this and may be in the next patch I will drop this check.
+ + 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 No. Impossible - I need connection variable virConnectPtr conn. And there is no connection variable in prlsdkAddFS
} - }
+ } confParam.nVmType = PVT_CT; confParam.sConfigSample = "vswap.1024MB"; confParam.nOsVersion = 0;