[libvirt] [PATCH rfc 0/2] devices: filesystem: type volume
In-Reply-To: Patches introduce the new type of filesystem device. The first patch adds the possibility to use storage pool volumes as a backend for filesystem, that is accesed from the guest. Such possibility allows to use volumes for container as a source of root directory. Usage: <devices> <filesystem type='volume'> <source pool='pool_name' volume='volume_name'/> ... <filesystem/> The second patch adds the support of filesystem type volume to virtuozzo containers. There is special type of volume for virtuozzo containers, that can be manipulated via storage api. Now, the volume of this type can be added to ct.xml description as filesystem source. However, there is a small issue with the pool/volume to path translation. Usuallyi, in storage pools, virStorageTranslateDiskSourcePool is used, but in the case of filesystem we do not have virDomainDiskDefPtr and and code deduplication in every driver,that uses this possibility, is unavoidable. eg. virStorageTranslatePoolLocal. Now I am looking for the way to avoid it somehow.
New type of <devices> <filesystem type= 'volume'> is introduced. This patch allows to use volumes for storing the filesystem, that is accessed from the guest e.g. root directory for container. To take advantage of volumes as a backend of filesystem volume and pool names should be specified: <filesystem type= 'volume'> <source pool='pool name' volume='volume name'/> Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/conf/domain_audit.c | 4 ++-- src/conf/domain_conf.c | 54 +++++++++++++++++++++++++++++++++++----------- src/conf/domain_conf.h | 4 +++- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_container.c | 50 +++++++++++++++++++++--------------------- src/lxc/lxc_controller.c | 18 ++++++++-------- src/lxc/lxc_native.c | 5 +++-- src/lxc/lxc_process.c | 2 +- src/openvz/openvz_conf.c | 4 ++-- src/openvz/openvz_driver.c | 4 ++-- src/qemu/qemu_command.c | 2 +- src/vbox/vbox_common.c | 6 +++--- src/vmx/vmx.c | 4 ++-- src/vz/vz_sdk.c | 10 ++++----- 15 files changed, 102 insertions(+), 68 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index bd2eeb6..dbfff91 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -290,8 +290,8 @@ virDomainAuditFS(virDomainObjPtr vm, const char *reason, bool success) { virDomainAuditGenericDev(vm, "fs", - oldDef ? oldDef->src : NULL, - newDef ? newDef->src : NULL, + oldDef ? oldDef->src->path : NULL, + newDef ? newDef->src->path : NULL, reason, success); } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 361e79e..0faf9d1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -354,7 +354,8 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, "file", "template", "ram", - "bind") + "bind", + "volume") VIR_ENUM_IMPL(virDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "default", @@ -1670,12 +1671,31 @@ void virDomainControllerDefFree(virDomainControllerDefPtr def) VIR_FREE(def); } +virDomainFSDefPtr +virDomainFSDefNew(void) +{ + virDomainFSDefPtr ret; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + if (VIR_ALLOC(ret->src) < 0) + goto cleanup; + + return ret; + + cleanup: + virDomainFSDefFree(ret); + return NULL; + +} + void virDomainFSDefFree(virDomainFSDefPtr def) { if (!def) return; - VIR_FREE(def->src); + virStorageSourceFree(def->src); VIR_FREE(def->dst); virDomainDeviceInfoClear(&def->info); @@ -8142,7 +8162,6 @@ virDomainNetGenerateMAC(virDomainXMLOptionPtr xmlopt, virMacAddrGenerate(xmlopt->config.macPrefix, mac); } - /* Parse the XML definition for a disk * @param node XML nodeset to parse for disk definition */ @@ -8165,7 +8184,7 @@ virDomainFSDefParseXML(xmlNodePtr node, ctxt->node = node; - if (VIR_ALLOC(def) < 0) + if (!(def = virDomainFSDefNew())) return NULL; type = virXMLPropString(node, "type"); @@ -8218,6 +8237,10 @@ virDomainFSDefParseXML(xmlNodePtr node, } else if (def->type == VIR_DOMAIN_FS_TYPE_RAM) { usage = virXMLPropString(cur, "usage"); units = virXMLPropString(cur, "units"); + } else if (def->type == VIR_DOMAIN_FS_TYPE_VOLUME) { + def->src->type = VIR_STORAGE_TYPE_VOLUME; + if (virDomainDiskSourcePoolDefParse(cur, &def->src->srcpool) < 0) + goto error; } } else if (!target && xmlStrEqual(cur->name, BAD_CAST "target")) { @@ -8262,8 +8285,8 @@ virDomainFSDefParseXML(xmlNodePtr node, def->wrpolicy = VIR_DOMAIN_FS_WRPOLICY_DEFAULT; } - if (source == NULL && - def->type != VIR_DOMAIN_FS_TYPE_RAM) { + if (source == NULL && def->type != VIR_DOMAIN_FS_TYPE_RAM + && def->type != VIR_DOMAIN_FS_TYPE_VOLUME) { virReportError(VIR_ERR_NO_SOURCE, target ? "%s" : NULL, target); goto error; @@ -8292,7 +8315,7 @@ virDomainFSDefParseXML(xmlNodePtr node, goto error; } - def->src = source; + def->src->path = source; source = NULL; def->dst = target; target = NULL; @@ -19397,6 +19420,7 @@ virDomainFSDefFormat(virBufferPtr buf, const char *accessmode = virDomainFSAccessModeTypeToString(def->accessmode); const char *fsdriver = virDomainFSDriverTypeToString(def->fsdriver); const char *wrpolicy = virDomainFSWrpolicyTypeToString(def->wrpolicy); + const char *src = def->src->path; if (!type) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -19433,30 +19457,36 @@ virDomainFSDefFormat(virBufferPtr buf, case VIR_DOMAIN_FS_TYPE_MOUNT: case VIR_DOMAIN_FS_TYPE_BIND: virBufferEscapeString(buf, "<source dir='%s'/>\n", - def->src); + src); break; case VIR_DOMAIN_FS_TYPE_BLOCK: virBufferEscapeString(buf, "<source dev='%s'/>\n", - def->src); + src); break; case VIR_DOMAIN_FS_TYPE_FILE: virBufferEscapeString(buf, "<source file='%s'/>\n", - def->src); + src); break; case VIR_DOMAIN_FS_TYPE_TEMPLATE: virBufferEscapeString(buf, "<source name='%s'/>\n", - def->src); + src); break; case VIR_DOMAIN_FS_TYPE_RAM: virBufferAsprintf(buf, "<source usage='%lld' units='KiB'/>\n", def->usage / 1024); break; - } + case VIR_DOMAIN_FS_TYPE_VOLUME: + virBufferAddLit(buf, "<source"); + virBufferEscapeString(buf, " pool='%s'", def->src->srcpool->pool); + virBufferEscapeString(buf, " volume='%s'", def->src->srcpool->volume); + virBufferAddLit(buf, "/>\n"); + break; + } virBufferEscapeString(buf, "<target dir='%s'/>\n", def->dst); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 83bdd67..7ebc494 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -859,6 +859,7 @@ typedef enum { VIR_DOMAIN_FS_TYPE_TEMPLATE, /* Expands a OS template to a guest dir */ VIR_DOMAIN_FS_TYPE_RAM, /* Mount a RAM filesystem on a guest dir */ VIR_DOMAIN_FS_TYPE_BIND, /* Binds a guest dir to another guest dir */ + VIR_DOMAIN_FS_TYPE_VOLUME, /* Mounts storage pool volume to a guest */ VIR_DOMAIN_FS_TYPE_LAST } virDomainFSType; @@ -902,7 +903,7 @@ struct _virDomainFSDef { int wrpolicy; /* enum virDomainFSWrpolicy */ int format; /* virStorageFileFormat */ unsigned long long usage; /* in bytes */ - char *src; + virStorageSourcePtr src; char *dst; bool readonly; virDomainDeviceInfo info; @@ -2529,6 +2530,7 @@ virDomainDiskDefPtr virDomainDiskFindByBusAndDst(virDomainDefPtr def, int bus, char *dst); void virDomainControllerDefFree(virDomainControllerDefPtr def); +virDomainFSDefPtr virDomainFSDefNew(void); void virDomainFSDefFree(virDomainFSDefPtr def); void virDomainActualNetDefFree(virDomainActualNetDefPtr def); void virDomainNetDefFree(virDomainNetDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1da256d..be5df1c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -288,6 +288,7 @@ virDomainDiskSetFormat; virDomainDiskSetSource; virDomainDiskSetType; virDomainDiskSourceIsBlockType; +virDomainFSDefNew; virDomainFSDefFree; virDomainFSIndexByName; virDomainFSInsert; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 4afe7e2..80c873d 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -411,7 +411,7 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, continue; if (virCgroupAllowDevicePath(cgroup, - def->fss[i]->src, + def->fss[i]->src->path, def->fss[i]->readonly ? VIR_CGROUP_DEVICE_READ : VIR_CGROUP_DEVICE_RW, false) < 0) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 348bbfb..a60ec87 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -649,27 +649,27 @@ static int lxcContainerResolveSymlinks(virDomainFSDefPtr fs, bool gentle) if (!fs->src || fs->symlinksResolved) return 0; - if (access(fs->src, F_OK)) { + if (access(fs->src->path, F_OK)) { if (gentle) { /* Just ignore the error for the while, we'll try again later */ - VIR_DEBUG("Skipped unaccessible '%s'", fs->src); + VIR_DEBUG("Skipped unaccessible '%s'", fs->src->path); return 0; } else { virReportSystemError(errno, - _("Failed to access '%s'"), fs->src); + _("Failed to access '%s'"), fs->src->path); return -1; } } - VIR_DEBUG("Resolving '%s'", fs->src); - if (virFileResolveAllLinks(fs->src, &newroot) < 0) { + VIR_DEBUG("Resolving '%s'", fs->src->path); + if (virFileResolveAllLinks(fs->src->path, &newroot) < 0) { if (gentle) { - VIR_DEBUG("Skipped non-resolvable '%s'", fs->src); + VIR_DEBUG("Skipped non-resolvable '%s'", fs->src->path); return 0; } else { virReportSystemError(errno, _("Failed to resolve symlink at %s"), - fs->src); + fs->src->path); } return -1; } @@ -677,10 +677,10 @@ static int lxcContainerResolveSymlinks(virDomainFSDefPtr fs, bool gentle) /* Mark it resolved to skip it the next time */ fs->symlinksResolved = true; - VIR_DEBUG("Resolved '%s' to %s", fs->src, newroot); + VIR_DEBUG("Resolved '%s' to %s", fs->src->path, newroot); - VIR_FREE(fs->src); - fs->src = newroot; + VIR_FREE(fs->src->path); + fs->src->path = newroot; return 0; } @@ -728,8 +728,8 @@ static int lxcContainerPrepareRoot(virDomainDefPtr def, root->dst = tmp; root->type = VIR_DOMAIN_FS_TYPE_MOUNT; - VIR_FREE(root->src); - root->src = dst; + VIR_FREE(root->src->path); + root->src->path = dst; return 0; } @@ -741,7 +741,7 @@ static int lxcContainerPivotRoot(virDomainFSDefPtr root) ret = -1; - VIR_DEBUG("Pivot via %s", root->src); + VIR_DEBUG("Pivot via %s", root->src->path); /* root->parent must be private, so make / private. */ if (mount("", "/", NULL, MS_PRIVATE|MS_REC, NULL) < 0) { @@ -750,7 +750,7 @@ static int lxcContainerPivotRoot(virDomainFSDefPtr root) goto err; } - if (virAsprintf(&oldroot, "%s/.oldroot", root->src) < 0) + if (virAsprintf(&oldroot, "%s/.oldroot", root->src->path) < 0) goto err; if (virFileMakePath(oldroot) < 0) { @@ -781,18 +781,18 @@ static int lxcContainerPivotRoot(virDomainFSDefPtr root) } /* ... and mount our root onto it */ - if (mount(root->src, newroot, NULL, MS_BIND|MS_REC, NULL) < 0) { + if (mount(root->src->path, newroot, NULL, MS_BIND|MS_REC, NULL) < 0) { virReportSystemError(errno, _("Failed to bind %s to new root %s"), - root->src, newroot); + root->src->path, newroot); goto err; } if (root->readonly) { - if (mount(root->src, newroot, NULL, MS_BIND|MS_REC|MS_RDONLY|MS_REMOUNT, NULL) < 0) { + if (mount(root->src->path, newroot, NULL, MS_BIND|MS_REC|MS_RDONLY|MS_REMOUNT, NULL) < 0) { virReportSystemError(errno, _("Failed to make new root %s readonly"), - root->src); + root->src->path); goto err; } } @@ -1209,9 +1209,9 @@ static int lxcContainerMountFSBind(virDomainFSDefPtr fs, int ret = -1; struct stat st; - VIR_DEBUG("src=%s dst=%s", fs->src, fs->dst); + VIR_DEBUG("src=%s dst=%s", fs->src->path, fs->dst); - if (virAsprintf(&src, "%s%s", srcprefix, fs->src) < 0) + if (virAsprintf(&src, "%s%s", srcprefix, fs->src->path) < 0) goto cleanup; if (stat(fs->dst, &st) < 0) { @@ -1544,9 +1544,9 @@ static int lxcContainerMountFSBlock(virDomainFSDefPtr fs, char *src = NULL; int ret = -1; - VIR_DEBUG("src=%s dst=%s", fs->src, fs->dst); + VIR_DEBUG("src=%s dst=%s", fs->src->path, fs->dst); - if (virAsprintf(&src, "%s%s", srcprefix, fs->src) < 0) + if (virAsprintf(&src, "%s%s", srcprefix, fs->src->path) < 0) goto cleanup; ret = lxcContainerMountFSBlockHelper(fs, src, srcprefix, sec_mount_options); @@ -1652,14 +1652,14 @@ static int lxcContainerMountAllFS(virDomainDefPtr vmDef, if (STREQ(vmDef->fss[i]->dst, "/")) continue; - VIR_DEBUG("Mounting '%s' -> '%s'", vmDef->fss[i]->src, vmDef->fss[i]->dst); + VIR_DEBUG("Mounting '%s' -> '%s'", vmDef->fss[i]->src->path, vmDef->fss[i]->dst); if (lxcContainerResolveSymlinks(vmDef->fss[i], false) < 0) return -1; if (!(vmDef->fss[i]->src && - STRPREFIX(vmDef->fss[i]->src, vmDef->fss[i]->dst)) && + STRPREFIX(vmDef->fss[i]->src->path, vmDef->fss[i]->dst)) && lxcContainerUnmountSubtree(vmDef->fss[i]->dst, false) < 0) return -1; @@ -1807,7 +1807,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, /* FIXME: we should find a way to unmount these mounts for container * even user namespace is enabled. */ - if (STREQ(root->src, "/") && (!vmDef->idmap.nuidmap) && + if (STREQ(root->src->path, "/") && (!vmDef->idmap.nuidmap) && lxcContainerUnmountForSharedRoot(stateDir, vmDef->name) < 0) goto cleanup; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 8b5ec4c..457724f 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -417,18 +417,18 @@ static int virLXCControllerSetupLoopDeviceFS(virDomainFSDefPtr fs) int lofd; char *loname = NULL; - if ((lofd = virFileLoopDeviceAssociate(fs->src, &loname)) < 0) + if ((lofd = virFileLoopDeviceAssociate(fs->src->path, &loname)) < 0) return -1; VIR_DEBUG("Changing fs %s to use type=block for dev %s", - fs->src, loname); + fs->src->path, loname); /* * We now change it into a block device type, so that * the rest of container setup 'just works' */ fs->type = VIR_DOMAIN_FS_TYPE_BLOCK; - VIR_FREE(fs->src); - fs->src = loname; + VIR_FREE(fs->src->path); + fs->src->path = loname; loname = NULL; return lofd; @@ -478,21 +478,21 @@ static int virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs) return -1; } - if (virFileNBDDeviceAssociate(fs->src, + if (virFileNBDDeviceAssociate(fs->src->path, fs->format, fs->readonly, &dev) < 0) return -1; VIR_DEBUG("Changing fs %s to use type=block for dev %s", - fs->src, dev); + fs->src->path, dev); /* * We now change it into a block device type, so that * the rest of container setup 'just works' */ fs->type = VIR_DOMAIN_FS_TYPE_BLOCK; - VIR_FREE(fs->src); - fs->src = dev; + VIR_FREE(fs->src->path); + fs->src->path = dev; return 0; } @@ -628,7 +628,7 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) /* The NBD device will be cleaned up while the cgroup will end. * For this we need to remember the qemu-nbd pid and add it to * the cgroup*/ - if (virLXCControllerAppendNBDPids(ctrl, fs->src) < 0) + if (virLXCControllerAppendNBDPids(ctrl, fs->src->path) < 0) goto cleanup; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index ef92c7d..53ce3f9 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -32,6 +32,7 @@ #include "util/virlog.h" #include "util/virstring.h" #include "util/virconf.h" +#include "conf/domain_conf.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -46,12 +47,12 @@ lxcCreateFSDef(int type, { virDomainFSDefPtr def; - if (VIR_ALLOC(def) < 0) + if (!(def = virDomainFSDefNew())) return NULL; def->type = type; def->accessmode = VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH; - if (src && VIR_STRDUP(def->src, src) < 0) + if (src && VIR_STRDUP(def->src->path, src) < 0) goto error; if (VIR_STRDUP(def->dst, dst) < 0) goto error; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 5e0bbe2..83ba0f3 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1135,7 +1135,7 @@ virLXCProcessEnsureRootFS(virDomainObjPtr vm) root->type = VIR_DOMAIN_FS_TYPE_MOUNT; - if (VIR_STRDUP(root->src, "/") < 0 || + if (VIR_STRDUP(root->src->path, "/") < 0 || VIR_STRDUP(root->dst, "/") < 0) goto error; diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 4103d69..b863f23 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -355,7 +355,7 @@ openvzReadFSConf(virDomainDefPtr def, goto error; fs->type = VIR_DOMAIN_FS_TYPE_TEMPLATE; - if (VIR_STRDUP(fs->src, temp) < 0) + if (VIR_STRDUP(fs->src->path, temp) < 0) goto error; } else { /* OSTEMPLATE was not found, VE was booted from a private dir directly */ @@ -374,7 +374,7 @@ openvzReadFSConf(virDomainDefPtr def, goto error; fs->type = VIR_DOMAIN_FS_TYPE_MOUNT; - if (!(fs->src = virStringReplace(temp, "$VEID", veid_str))) + if (!(fs->src->path = virStringReplace(temp, "$VEID", veid_str))) goto error; VIR_FREE(veid_str); diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index e154a0f..ef489c3 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -203,7 +203,7 @@ static int openvzSetInitialConfig(virDomainDefPtr vmdef) goto cleanup; } - if (openvzWriteVPSConfigParam(vpsid, "VE_PRIVATE", vmdef->fss[0]->src) < 0) { + if (openvzWriteVPSConfigParam(vpsid, "VE_PRIVATE", vmdef->fss[0]->src->path) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set the source dir for the filesystem")); goto cleanup; @@ -2047,7 +2047,7 @@ openvzUpdateDevice(virDomainDefPtr vmdef, cur = vmdef->fss[pos]; /* We only allow updating the quota */ - if (STRNEQ(cur->src, fs->src) + if (STRNEQ(cur->src->path, fs->src->path) || cur->type != fs->type || cur->accessmode != fs->accessmode || cur->wrpolicy != fs->wrpolicy diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eb02553..1483512 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2033,7 +2033,7 @@ qemuBuildFSStr(virDomainFSDefPtr fs, } virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); - virBufferAsprintf(&opt, ",path=%s", fs->src); + virBufferAsprintf(&opt, ",path=%s", fs->src->path); if (fs->readonly) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_READONLY)) { diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index c305eb5..7f76ab7 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1837,7 +1837,7 @@ vboxAttachSharedFolder(virDomainDefPtr def, vboxGlobalData *data, IMachine *mach continue; VBOX_UTF8_TO_UTF16(def->fss[i]->dst, &nameUtf16); - VBOX_UTF8_TO_UTF16(def->fss[i]->src, &hostPathUtf16); + VBOX_UTF8_TO_UTF16(def->fss[i]->src->path, &hostPathUtf16); writable = !def->fss[i]->readonly; gVBoxAPI.UIMachine.CreateSharedFolder(machine, nameUtf16, hostPathUtf16, @@ -3445,7 +3445,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxGlobalData *data, IMachine *machi gVBoxAPI.UISharedFolder.GetHostPath(sharedFolder, &hostPathUtf16); VBOX_UTF16_TO_UTF8(hostPathUtf16, &hostPath); - if (VIR_STRDUP(def->fss[i]->src, hostPath) < 0) { + if (VIR_STRDUP(def->fss[i]->src->path, hostPath) < 0) { VBOX_UTF8_FREE(hostPath); VBOX_UTF16_FREE(hostPathUtf16); goto sharedFoldersCleanup; @@ -4156,7 +4156,7 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom, PRBool writable; VBOX_UTF8_TO_UTF16(dev->data.fs->dst, &nameUtf16); - VBOX_UTF8_TO_UTF16(dev->data.fs->src, &hostPathUtf16); + VBOX_UTF8_TO_UTF16(dev->data.fs->src->path, &hostPathUtf16); writable = !dev->data.fs->readonly; rc = gVBoxAPI.UIMachine.CreateSharedFolder(machine, nameUtf16, hostPathUtf16, diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index f77a7a4..e350bed 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2451,7 +2451,7 @@ int virVMXParseFileSystem(virConfPtr conf, int number, virDomainFSDefPtr *def) if (virVMXGetConfigString(conf, hostPath_name, &hostPath, false) < 0) goto cleanup; - (*def)->src = hostPath; + (*def)->src->path = hostPath; hostPath = NULL; /* vmx:guestName */ @@ -3663,7 +3663,7 @@ virVMXFormatFileSystem(virDomainFSDefPtr def, int number, virBufferPtr buffer) virBufferAsprintf(buffer, "sharedFolder%d.writeAccess = \"%s\"\n", number, def->readonly ? "false" : "true"); virBufferAsprintf(buffer, "sharedFolder%d.hostPath = \"%s\"\n", number, - def->src); + def->src->path); virBufferAsprintf(buffer, "sharedFolder%d.guestName = \"%s\"\n", number, def->dst); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 8691887..00f42b8 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -589,7 +589,7 @@ prlsdkGetFSInfo(PRL_HANDLE prldisk, pret = PrlVmDev_GetImagePath(prldisk, buf, &buflen); prlsdkCheckRetGoto(pret, cleanup); - fs->src = buf; + fs->src->path = buf; buf = NULL; pret = PrlVmDevHd_GetMountPoint(prldisk, NULL, &buflen); @@ -3282,13 +3282,13 @@ prlsdkAddFS(PRL_HANDLE sdkdom, virDomainFSDefPtr fs) pret = PrlVmDev_SetEmulatedType(sdkdisk, PDT_USE_IMAGE_FILE); prlsdkCheckRetGoto(pret, cleanup); - pret = PrlVmDev_SetSysName(sdkdisk, fs->src); + pret = PrlVmDev_SetSysName(sdkdisk, fs->src->path); prlsdkCheckRetGoto(pret, cleanup); - pret = PrlVmDev_SetImagePath(sdkdisk, fs->src); + pret = PrlVmDev_SetImagePath(sdkdisk, fs->src->path); prlsdkCheckRetGoto(pret, cleanup); - pret = PrlVmDev_SetFriendlyName(sdkdisk, fs->src); + pret = PrlVmDev_SetFriendlyName(sdkdisk, fs->src->path); prlsdkCheckRetGoto(pret, cleanup); pret = PrlVmDevHd_SetMountPoint(sdkdisk, fs->dst); @@ -3537,7 +3537,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) prlsdkCheckRetGoto(pret, cleanup); if (useTemplate) { - pret = PrlVmCfg_SetOsTemplate(sdkdom, def->fss[0]->src); + pret = PrlVmCfg_SetOsTemplate(sdkdom, def->fss[0]->src->path); prlsdkCheckRetGoto(pret, cleanup); } -- 1.8.3.1
On Fri, Apr 08, 2016 at 07:39:23PM +0300, Olga Krishtal wrote:
New type of <devices> <filesystem type= 'volume'> is introduced. This patch allows to use volumes for storing the filesystem, that is accessed from the guest e.g. root directory for container.
To take advantage of volumes as a backend of filesystem volume and pool names should be specified: <filesystem type= 'volume'> <source pool='pool name' volume='volume name'/>
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/conf/domain_audit.c | 4 ++-- src/conf/domain_conf.c | 54 +++++++++++++++++++++++++++++++++++----------- src/conf/domain_conf.h | 4 +++- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_container.c | 50 +++++++++++++++++++++--------------------- src/lxc/lxc_controller.c | 18 ++++++++-------- src/lxc/lxc_native.c | 5 +++-- src/lxc/lxc_process.c | 2 +- src/openvz/openvz_conf.c | 4 ++-- src/openvz/openvz_driver.c | 4 ++-- src/qemu/qemu_command.c | 2 +- src/vbox/vbox_common.c | 6 +++--- src/vmx/vmx.c | 4 ++-- src/vz/vz_sdk.c | 10 ++++----- 15 files changed, 102 insertions(+), 68 deletions(-)
Looks good to me, but I do not feel competent to comment on the vz* part. Also, could you please separate the changes adapting from ->src to ->src->path from the changes adding the new type? Jan
On 23/06/16 15:31, Ján Tomko wrote:
On Fri, Apr 08, 2016 at 07:39:23PM +0300, Olga Krishtal wrote:
New type of <devices> <filesystem type= 'volume'> is introduced. This patch allows to use volumes for storing the filesystem, that is accessed from the guest e.g. root directory for container.
To take advantage of volumes as a backend of filesystem volume and pool names should be specified: <filesystem type= 'volume'> <source pool='pool name' volume='volume name'/>
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/conf/domain_audit.c | 4 ++-- src/conf/domain_conf.c | 54 +++++++++++++++++++++++++++++++++++----------- src/conf/domain_conf.h | 4 +++- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_container.c | 50 +++++++++++++++++++++--------------------- src/lxc/lxc_controller.c | 18 ++++++++-------- src/lxc/lxc_native.c | 5 +++-- src/lxc/lxc_process.c | 2 +- src/openvz/openvz_conf.c | 4 ++-- src/openvz/openvz_driver.c | 4 ++-- src/qemu/qemu_command.c | 2 +- src/vbox/vbox_common.c | 6 +++--- src/vmx/vmx.c | 4 ++-- src/vz/vz_sdk.c | 10 ++++----- 15 files changed, 102 insertions(+), 68 deletions(-)
Looks good to me, but I do not feel competent to comment on the vz* part.
Also, could you please separate the changes adapting from ->src to ->src->path from the changes adding the new type?
Jan
Ok, will rebase, split and add comment
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" +#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); + + if (!virStringIsEmpty(storage)) { + uri = virURIParse(storage); + if (!uri || STRNEQ("volume", uri->scheme)) + goto cleanup; + if (!(matches = virStringSplitCount(uri->path, "/", 0, NULL))) + goto cleanup; + 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; + } - 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; } + 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, + 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; } 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; + + 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"; confParam.nOsVersion = 0; -- 1.8.3.1
08.04.2016 19:39, Olga Krishtal пишет:
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>
typo: specified
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(-)
Looks ok but needs to be rebased. Also I think we need to note at least in comments what kind of links we use in PrlVmDevHd_GetStorageURL Maxim
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
+ + 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;
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;
On 08/04/16 19:39, Olga Krishtal wrote:
In-Reply-To:
Patches introduce the new type of filesystem device.
The first patch adds the possibility to use storage pool volumes as a backend for filesystem, that is accesed from the guest. Such possibility allows to use volumes for container as a source of root directory.
Usage: <devices> <filesystem type='volume'> <source pool='pool_name' volume='volume_name'/> ... <filesystem/>
The second patch adds the support of filesystem type volume to virtuozzo containers. There is special type of volume for virtuozzo containers, that can be manipulated via storage api. Now, the volume of this type can be added to ct.xml description as filesystem source.
However, there is a small issue with the pool/volume to path translation. Usuallyi, in storage pools, virStorageTranslateDiskSourcePool is used, but in the case of filesystem we do not have virDomainDiskDefPtr and and code deduplication in every driver,that uses this possibility, is unavoidable. eg. virStorageTranslatePoolLocal. Now I am looking for the way to avoid it somehow.
ping
participants (4)
-
Ján Tomko -
Maxim Nestratov -
Nikolay Shirokovskiy -
Olga Krishtal