On 04/18/2012 02:07 PM, Dmitry Guryanov wrote:
PVS has one serious discrepancy with libvirt: libvirt stores
domain configuration files always in one place, and storage files
in other places (with API of storage pools and storage volumes).
PVS store all domain data in a single directory, for example, you
may have domain with name fedora-15, which will be located in
'/var/parallels/fedora-15.pvm', and it's hard disk image will be
in '/var/parallels/fedora-15.pvm/harddisk1.hdd'.
I've decided to create storage driver, which produces pseudo-volumes
(xml files with volume description), and they will be 'converted' to
real disk images after attaching to a VM.
So if someone creates VM with one hard disk using virt-manager,
at first virt-manager creates a new volume, and then defines a
domain. We can lookup a volume by path in XML domain definition
and find out location of new domain and size of its hard disk.
This code mostly duplicates code in libvirt's default storage
driver, but I haven't found, how functions from that driver can
be reused. So if it possible I'll be very grateful for the advice,
how to do it.
[...]
+
+static int
+pvsFindVolumes(virStoragePoolObjPtr pool)
+{
+ int ret;
+ DIR *dir;
+ struct dirent *ent;
+ char *path;
+
+ if (!(dir = opendir(pool->def->target.path))) {
+ virReportSystemError(errno,
+ _("cannot open path '%s'"),
+ pool->def->target.path);
+ goto cleanup;
Need to set ret = -1.
+ }
+
+ while ((ent = readdir(dir)) != NULL) {
+ if (!virFileHasSuffix(ent->d_name, ".xml"))
+ continue;
+
+ if (!(path = virFileBuildPath(pool->def->target.path,
+ ent->d_name, NULL)))
+ goto no_memory;
+ if (!pvsStorageVolumeDefine(pool, NULL, path, false))
+ goto cleanup;
+ VIR_FREE(path);
+ }
+
Nothing assigns a value to ret...
+ return ret;
+ no_memory:
+ virReportOOMError();
+ cleanup:
+ return ret;
+
+}
+
+static virDrvOpenStatus
+pvsStorageOpen(virConnectPtr conn,
+ virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags)
+{
+ char *base = NULL;
+ virStorageDriverStatePtr storageState;
+ int privileged = (geteuid() == 0);
+ pvsConnPtr privconn = conn->privateData;
+ virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
+
+ if (STRNEQ(conn->driver->name, "PVS"))
+ return VIR_DRV_OPEN_DECLINED;
+
+ if (VIR_ALLOC(storageState)< 0)
+ return VIR_DRV_OPEN_ERROR;
virReportOOMError() ?
+
+ if (virMutexInit(&storageState->lock)< 0) {
+ VIR_FREE(storageState);
+ return VIR_DRV_OPEN_ERROR;
+ }
+ pvsStorageLock(storageState);
+
+ if (privileged) {
+ if ((base = strdup(SYSCONFDIR "/libvirt")) == NULL)
+ goto out_of_memory;
+ } else {
+ uid_t uid = geteuid();
+
+ char *userdir = virGetUserDirectory(uid);
+
+ if (!userdir)
+ goto error;
+
+ if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) {
+ VIR_FREE(userdir);
+ goto out_of_memory;
+ }
+ VIR_FREE(userdir);
+ }
+
+ /* Configuration paths are either ~/.libvirt/storage/... (session) or
+ * /etc/libvirt/storage/... (system).
+ */
+ if (virAsprintf(&storageState->configDir,
+ "%s/pvs-storage", base) == -1)
+ goto out_of_memory;
+
+ if (virAsprintf(&storageState->autostartDir,
+ "%s/pvs-storage/autostart", base) == -1)
+ goto out_of_memory;
+
+ VIR_FREE(base);
+
+ if (virStoragePoolLoadAllConfigs(&privconn->pools,
+ storageState->configDir,
+ storageState->autostartDir)< 0) {
+ pvsError(VIR_ERR_INTERNAL_ERROR, _("Failed to load pool configs"));
+ goto error;
+ }
+
+ for (int i = 0; i< privconn->pools.count; i++) {
+ virStoragePoolObjLock(privconn->pools.objs[i]);
+ virStoragePoolObjPtr pool;
+
+ pool = privconn->pools.objs[i];
+ if (pool->autostart)
+ pool->active = 1;
+ pool->active = 1;
pool is always active ? test before seems redundant
+
+ if (pvsStoragePoolGetAlloc(pool->def))
< 0 check
+ goto error;
+
+ if (pvsFindVolumes(pool))
< 0 check ?
+ goto error;
+
+ virStoragePoolObjUnlock(privconn->pools.objs[i]);
+ }
+
+ pvsStorageUnlock(storageState);
+
+ conn->storagePrivateData = storageState;
+
+ return VIR_DRV_OPEN_SUCCESS;
+
+ out_of_memory:
+ virReportOOMError();
+ error:
+ VIR_FREE(base);
+ pvsStorageUnlock(storageState);
+ pvsStorageClose(conn);
+ return -1;
+}
+
+static int
+pvsStorageClose(virConnectPtr conn)
+{
+ pvsConnPtr privconn = conn->privateData;
+ virStorageDriverStatePtr storageState = conn->storagePrivateData;
add empty line here after var decl.
+ conn->storagePrivateData = NULL;
+
+ pvsStorageLock(storageState);
+ virStoragePoolObjListFree(&privconn->pools);+
+/*
+ * Fill capacity, available and allocation
+ * fields in pool definition.
+ */
+static int
+pvsStoragePoolGetAlloc(virStoragePoolDefPtr def)
+{
+ struct statvfs sb;
+
Different spacing here?
+ if (statvfs(def->target.path,&sb)< 0) {
+ virReportSystemError(errno,
+ _("cannot statvfs path '%s'"),
+ def->target.path);
+ return -1;
+ }
+
+ def->capacity = ((unsigned long long)sb.f_frsize *
+ (unsigned long long)sb.f_blocks);
+ def->available = ((unsigned long long)sb.f_bfree *
+ (unsigned long long)sb.f_bsize);
+ def->allocation = def->capacity - def->available;
+
+ return 0;
+}
+
+static virStoragePoolPtr
+pvsStoragePoolDefine(virConnectPtr conn,
+ const char *xml, unsigned int flags)
+{
[...]
+
+ if (virStoragePoolSourceFindDuplicate(&privconn->pools, def)< 0)
+ goto cleanup;
+
+ if (pvsStoragePoolGetAlloc(def))
+ goto cleanup;
Spacing...
+pvsStoragePoolDestroy(virStoragePoolPtr pool)
+{
+ pvsConnPtr privconn = pool->conn->privateData;
+ virStoragePoolObjPtr privpool;
+ int ret = -1;
+
+ pvsDriverLock(privconn);
+ privpool = virStoragePoolObjFindByName(&privconn->pools, pool->name);
+
+ if (privpool == NULL) {
+ pvsError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ goto cleanup;
+ }
+
+ if (!virStoragePoolObjIsActive(privpool)) {
+ pvsError(VIR_ERR_OPERATION_INVALID,
+ _("storage pool '%s' is not active"), pool->name);
+ goto cleanup;
+ }
+
+ privpool->active = 0;
+
No need to set this...
+pvsStorageVolumeDefine(virStoragePoolObjPtr pool,
+ const char *xmldesc,
+ const char *xmlfile, bool is_new)
+{
+ virStorageVolDefPtr privvol = NULL;
+ virStorageVolDefPtr ret = NULL;
+ char *xml_path = NULL;
+
+ if (xmlfile)
+ privvol = virStorageVolDefParseFile(pool->def, xmlfile);
+ else
+ privvol = virStorageVolDefParseString(pool->def, xmldesc);
+ if (privvol == NULL)
+ goto cleanup;
+
+ if (virStorageVolDefFindByName(pool, privvol->name)) {
+ pvsError(VIR_ERR_OPERATION_FAILED,
+ "%s", _("storage vol already exists"));
+ goto cleanup;
+ }
+
+ if (is_new) {
+ /* Make sure enough space */
+ if ((pool->def->allocation + privvol->allocation)>
+ pool->def->capacity) {
+ pvsError(VIR_ERR_INTERNAL_ERROR,
+ _("Not enough free space in pool for volume '%s'"),
+ privvol->name);
+ goto cleanup;
+ }
+ }
+
+ if (VIR_REALLOC_N(pool->volumes.objs, pool->volumes.count + 1)< 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (virAsprintf(&privvol->target.path, "%s/%s",
+ pool->def->target.path, privvol->name) == -1) {
Nit: < 0
+static virStorageVolPtr
+pvsStorageVolumeCreateXML(virStoragePoolPtr pool,
+ const char *xmldesc, unsigned int flags)
+{
+ pvsConnPtr privconn = pool->conn->privateData;
+ virStoragePoolObjPtr privpool;
+ virStorageVolPtr ret = NULL;
+ virStorageVolDefPtr privvol = NULL;
+
+ virCheckFlags(0, NULL);
+
+ pvsDriverLock(privconn);
+ privpool = virStoragePoolObjFindByName(&privconn->pools, pool->name);
+ pvsDriverUnlock(privconn);
+
+ if (privpool == NULL) {
+ pvsError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ goto cleanup;
+ }
+
+ if (!virStoragePoolObjIsActive(privpool)) {
+ pvsError(VIR_ERR_OPERATION_INVALID,
+ _("storage pool '%s' is not active"), pool->name);
+ goto cleanup;
+ }
+
+ privvol = pvsStorageVolumeDefine(privpool, xmldesc, NULL, true);
+
NULL pointer test?
+/*
+ * Return new file path in malloced string created by
+ * concatenating first and second function arguments.
+ */
+char *
+pvsAddFileExt(const char *path, const char *ext)
+{
+ char *new_path = NULL;
+ size_t len = strlen(path) + strlen(ext) + 1;
+
+ if (VIR_ALLOC_N(new_path, len)< 0)
+ return NULL;
+
Nit: virReportOOMError() right where it happens rather than at the callers
+ if (!virStrcpy(new_path, path, len))
+ return NULL;
+ strcat(new_path, ext);
+
+ return new_path;
+}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list