[libvirt] [PATCH 2/2] vz: support filesystem type volume
Olga Krishtal
okrishtal at virtuozzo.com
Wed Jun 29 13:50:42 UTC 2016
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 at 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;
> indentation
Ok
>> - } 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.
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;
>>
More information about the libvir-list
mailing list