On 07/20/2012 07:34 PM, Peter Krempa wrote:
On 07/04/12 19:42, Dmitry Guryanov wrote:PARALLELS has one serious discrepancy with libvirt: libvirt stores domain configuration files in one place, and storage files in other places (with the API of storage pools and storage volumes). PARALLELS stores 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. Signed-off-by: Dmitry Guryanov <dgurya...@parallels.com> ---Comments inline.
Hello, Peter, Thank you very much for review ! This storage driver isn't completely working yet, it just allows to create a new VM using virt-manager or virsh. I'm going to add support of hard disk devices to parallels_driver.c, then it will be possible to populate actual list of volumes. Now this driver can operate on the volume definitions only.
po/POTFILES.in | 1 + src/Makefile.am | 3 +- src/parallels/parallels_driver.c | 6 +- src/parallels/parallels_driver.h | 5 +src/parallels/parallels_storage.c | 1460 +++++++++++++++++++++++++++++++++++++src/parallels/parallels_utils.c | 24 + 6 files changed, 1496 insertions(+), 3 deletions(-) create mode 100644 src/parallels/parallels_storage.c diff --git a/po/POTFILES.in b/po/POTFILES.in index dcb0813..240becb 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -66,6 +66,7 @@ src/phyp/phyp_driver.c
......
+ goto cleanup; + } + + if (virStoragePoolObjIsActive(privpool)) { + parallelsError(VIR_ERR_OPERATION_INVALID, + _("storage pool '%s' is already active"), pool->name); + goto cleanup; + }Also this function doesn't appear to be doing what it is supposed to do.
I'll remove it from this patch.
+ + ret = 0; + + cleanup: + if (privpool) + virStoragePoolObjUnlock(privpool); + return ret; +} + + +static int +parallelsStoragePoolRefresh(virStoragePoolPtr pool, unsigned int flags) +{ + parallelsConnPtr privconn = pool->conn->privateData; + virStoragePoolObjPtr privpool; + int ret = -1; + + virCheckFlags(0, -1); + + parallelsDriverLock(privconn);+ privpool = virStoragePoolObjFindByName(&privconn->pools, pool->name);+ parallelsDriverUnlock(privconn); + + if (privpool == NULL) { + parallelsError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto cleanup; + } + + if (!virStoragePoolObjIsActive(privpool)) { + parallelsError(VIR_ERR_OPERATION_INVALID, + _("storage pool '%s' is not active"), pool->name); + goto cleanup; + } + ret = 0;Same here.
And this function too.
.....+ + cleanup: + if (privpool) + virStoragePoolObjUnlock(privpool); + return ret; +} + + +static virStorageVolPtr +parallelsStorageVolumeCreateXMLFrom(virStoragePoolPtr pool, + const char *xmldesc, + virStorageVolPtr clonevol, + unsigned int flags) +{ + parallelsConnPtr privconn = pool->conn->privateData; + virStoragePoolObjPtr privpool; + virStorageVolDefPtr privvol = NULL, origvol = NULL; + virStorageVolPtr ret = NULL; + + virCheckFlags(0, NULL); + + parallelsDriverLock(privconn);+ privpool = virStoragePoolObjFindByName(&privconn->pools, pool->name);+ parallelsDriverUnlock(privconn); + + if (privpool == NULL) { + parallelsError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto cleanup; + } + + if (!virStoragePoolObjIsActive(privpool)) { + parallelsError(VIR_ERR_OPERATION_INVALID, + _("storage pool '%s' is not active"), pool->name); + goto cleanup; + } + + privvol = virStorageVolDefParseString(privpool->def, xmldesc); + if (privvol == NULL) + goto cleanup; + + if (virStorageVolDefFindByName(privpool, privvol->name)) { + parallelsError(VIR_ERR_OPERATION_FAILED, + "%s", _("storage vol already exists")); + goto cleanup; + } + + origvol = virStorageVolDefFindByName(privpool, clonevol->name); + if (!origvol) { + parallelsError(VIR_ERR_NO_STORAGE_VOL, + _("no storage vol with matching name '%s'"), + clonevol->name); + goto cleanup; + } + + /* Make sure enough space */ + if ((privpool->def->allocation + privvol->allocation) > + privpool->def->capacity) { + parallelsError(VIR_ERR_INTERNAL_ERROR, + _("Not enough free space in pool for volume '%s'"), + privvol->name); + goto cleanup; + } + privpool->def->available = (privpool->def->capacity - + privpool->def->allocation); + + if (VIR_REALLOC_N(privpool->volumes.objs, + privpool->volumes.count + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&privvol->target.path, "%s/%s", + privpool->def->target.path, privvol->name) == -1) { + virReportOOMError(); + goto cleanup; + } + + privvol->key = strdup(privvol->target.path); + if (privvol->key == NULL) { + virReportOOMError(); + goto cleanup; + } + + privpool->def->allocation += privvol->allocation; + privpool->def->available = (privpool->def->capacity - + privpool->def->allocation); + + privpool->volumes.objs[privpool->volumes.count++] = privvol;This function copies the volume definition but does not actualy copy data, is that ok?
It seems this function isn't needed, I'll remove it.
+ + ret = virGetStorageVol(pool->conn, privpool->def->name, + privvol->name, privvol->key); + privvol = NULL; + + cleanup: + virStorageVolDefFree(privvol); + if (privpool) + virStoragePoolObjUnlock(privpool); + return ret; +} + +static int +parallelsStorageVolumeDelete(virStorageVolPtr vol, unsigned int flags) +{ + parallelsConnPtr privconn = vol->conn->privateData; + virStoragePoolObjPtr privpool; + virStorageVolDefPtr privvol; + int i; + int ret = -1; + char *xml_path = NULL; + + virCheckFlags(0, -1); + + parallelsDriverLock(privconn);+ privpool = virStoragePoolObjFindByName(&privconn->pools, vol->pool);+ parallelsDriverUnlock(privconn); + + if (privpool == NULL) { + parallelsError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto cleanup; + } + + + privvol = virStorageVolDefFindByName(privpool, vol->name); + + if (privvol == NULL) { + parallelsError(VIR_ERR_NO_STORAGE_VOL,+ _("no storage vol with matching name '%s'"), vol->name);+ goto cleanup; + } + + if (!virStoragePoolObjIsActive(privpool)) { + parallelsError(VIR_ERR_OPERATION_INVALID, + _("storage pool '%s' is not active"), vol->pool); + goto cleanup; + } + + + privpool->def->allocation -= privvol->allocation; + privpool->def->available = (privpool->def->capacity - + privpool->def->allocation); + + for (i = 0; i < privpool->volumes.count; i++) { + if (privpool->volumes.objs[i] == privvol) {+ xml_path = parallelsAddFileExt(privvol->target.path, ".xml");+ if (!xml_path) + goto cleanup; + + if (unlink(xml_path)) { + parallelsError(VIR_ERR_OPERATION_FAILED, + _("Can't remove file '%s'"), xml_path); + goto cleanup; + } + + virStorageVolDefFree(privvol); + + if (i < (privpool->volumes.count - 1)) + memmove(privpool->volumes.objs + i, + privpool->volumes.objs + i + 1, + sizeof(*(privpool->volumes.objs)) * + (privpool->volumes.count - (i + 1))); + + if (VIR_REALLOC_N(privpool->volumes.objs, + privpool->volumes.count - 1) < 0) {+ ; /* Failure to reduce memory allocation isn't fatal */+ } + privpool->volumes.count--; + + break; + } + }Same here. You remove the definition but you don't touch the data.
Yes, this is OK for now.
+ ret = 0; + + cleanup: + if (privpool) + virStoragePoolObjUnlock(privpool); + VIR_FREE(xml_path); + return ret; +} + + +static int +parallelsStorageVolumeTypeForPool(int pooltype) +{ + + switch (pooltype) { + case VIR_STORAGE_POOL_DIR: + case VIR_STORAGE_POOL_FS: + case VIR_STORAGE_POOL_NETFS: + return VIR_STORAGE_VOL_FILE; + default: + return VIR_STORAGE_VOL_BLOCK; + } +} + +static int+parallelsStorageVolumeGetInfo(virStorageVolPtr vol, virStorageVolInfoPtr info)+{ + parallelsConnPtr privconn = vol->conn->privateData; + virStoragePoolObjPtr privpool; + virStorageVolDefPtr privvol; + int ret = -1; + + parallelsDriverLock(privconn);+ privpool = virStoragePoolObjFindByName(&privconn->pools, vol->pool);+ parallelsDriverUnlock(privconn); + + if (privpool == NULL) { + parallelsError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto cleanup; + } + + privvol = virStorageVolDefFindByName(privpool, vol->name); + + if (privvol == NULL) { + parallelsError(VIR_ERR_NO_STORAGE_VOL,+ _("no storage vol with matching name '%s'"), vol->name);+ goto cleanup; + } + + if (!virStoragePoolObjIsActive(privpool)) { + parallelsError(VIR_ERR_OPERATION_INVALID, + _("storage pool '%s' is not active"), vol->pool); + goto cleanup; + } + + memset(info, 0, sizeof(*info));+ info->type = parallelsStorageVolumeTypeForPool(privpool->def->type);+ info->capacity = privvol->capacity; + info->allocation = privvol->allocation; + ret = 0; + + cleanup: + if (privpool) + virStoragePoolObjUnlock(privpool); + return ret; +} + +static char *+parallelsStorageVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags)+{ + parallelsConnPtr privconn = vol->conn->privateData; + virStoragePoolObjPtr privpool; + virStorageVolDefPtr privvol; + char *ret = NULL; + + virCheckFlags(0, NULL); + + parallelsDriverLock(privconn);+ privpool = virStoragePoolObjFindByName(&privconn->pools, vol->pool);+ parallelsDriverUnlock(privconn); + + if (privpool == NULL) { + parallelsError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto cleanup; + } + + privvol = virStorageVolDefFindByName(privpool, vol->name); + + if (privvol == NULL) { + parallelsError(VIR_ERR_NO_STORAGE_VOL,+ _("no storage vol with matching name '%s'"), vol->name);+ goto cleanup; + } + + if (!virStoragePoolObjIsActive(privpool)) { + parallelsError(VIR_ERR_OPERATION_INVALID, + _("storage pool '%s' is not active"), vol->pool); + goto cleanup; + } + + ret = virStorageVolDefFormat(privpool->def, privvol); + + cleanup: + if (privpool) + virStoragePoolObjUnlock(privpool); + return ret; +} + +static char * +parallelsStorageVolumeGetPath(virStorageVolPtr vol) +{ + parallelsConnPtr privconn = vol->conn->privateData; + virStoragePoolObjPtr privpool; + virStorageVolDefPtr privvol; + char *ret = NULL; + + parallelsDriverLock(privconn);+ privpool = virStoragePoolObjFindByName(&privconn->pools, vol->pool);+ parallelsDriverUnlock(privconn); + + if (privpool == NULL) { + parallelsError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto cleanup; + } + + privvol = virStorageVolDefFindByName(privpool, vol->name); + + if (privvol == NULL) { + parallelsError(VIR_ERR_NO_STORAGE_VOL,+ _("no storage vol with matching name '%s'"), vol->name);+ goto cleanup; + } + + if (!virStoragePoolObjIsActive(privpool)) { + parallelsError(VIR_ERR_OPERATION_INVALID, + _("storage pool '%s' is not active"), vol->pool); + goto cleanup; + } + + ret = strdup(privvol->target.path); + if (ret == NULL) + virReportOOMError(); + + cleanup: + if (privpool) + virStoragePoolObjUnlock(privpool); + return ret; +} + +static virStorageDriver parallelsStorageDriver = { + .name = "PARALLELS", + .open = parallelsStorageOpen, /* 0.10.0 */ + .close = parallelsStorageClose, /* 0.10.0 */ + + .numOfPools = parallelsStorageNumPools, /* 0.10.0 */ + .listPools = parallelsStorageListPools, /* 0.10.0 */ + .numOfDefinedPools = parallelsStorageNumDefinedPools, /* 0.10.0 */ + .listDefinedPools = parallelsStorageListDefinedPools, /* 0.10.0 */ + .findPoolSources = parallelsStorageFindPoolSources, /* 0.10.0 */ + .poolLookupByName = parallelsStoragePoolLookupByName, /* 0.10.0 */ + .poolLookupByUUID = parallelsStoragePoolLookupByUUID, /* 0.10.0 */+ .poolLookupByVolume = parallelsStoragePoolLookupByVolume, /* 0.10.0 */+ .poolDefineXML = parallelsStoragePoolDefine, /* 0.10.0 */ + .poolBuild = parallelsStoragePoolBuild, /* 0.10.0 */ + .poolUndefine = parallelsStoragePoolUndefine, /* 0.10.0 */ + .poolCreate = parallelsStoragePoolStart, /* 0.10.0 */ + .poolDestroy = parallelsStoragePoolDestroy, /* 0.10.0 */ + .poolDelete = parallelsStoragePoolDelete, /* 0.10.0 */ + .poolRefresh = parallelsStoragePoolRefresh, /* 0.10.0 */ + .poolGetInfo = parallelsStoragePoolGetInfo, /* 0.10.0 */ + .poolGetXMLDesc = parallelsStoragePoolGetXMLDesc, /* 0.10.0 */ + .poolGetAutostart = parallelsStoragePoolGetAutostart, /* 0.10.0 */ + .poolSetAutostart = parallelsStoragePoolSetAutostart, /* 0.10.0 */ + .poolNumOfVolumes = parallelsStoragePoolNumVolumes, /* 0.10.0 */ + .poolListVolumes = parallelsStoragePoolListVolumes, /* 0.10.0 */ + + .volLookupByName = parallelsStorageVolumeLookupByName, /* 0.10.0 */ + .volLookupByKey = parallelsStorageVolumeLookupByKey, /* 0.10.0 */ + .volLookupByPath = parallelsStorageVolumeLookupByPath, /* 0.10.0 */ + .volCreateXML = parallelsStorageVolumeCreateXML, /* 0.10.0 */+ .volCreateXMLFrom = parallelsStorageVolumeCreateXMLFrom, /* 0.10.0 */+ .volDelete = parallelsStorageVolumeDelete, /* 0.10.0 */ + .volGetInfo = parallelsStorageVolumeGetInfo, /* 0.10.0 */ + .volGetXMLDesc = parallelsStorageVolumeGetXMLDesc, /* 0.10.0 */ + .volGetPath = parallelsStorageVolumeGetPath, /* 0.10.0 */ + .poolIsActive = parallelsStoragePoolIsActive, /* 0.10.0 */ + .poolIsPersistent = parallelsStoragePoolIsPersistent, /* 0.10.0 */ +}; + +int +parallelsStorageRegister(void) +{ + if (virRegisterStorageDriver(¶llelsStorageDriver) < 0) + return -1; + + return 0; +}diff --git a/src/parallels/parallels_utils.c b/src/parallels/parallels_utils.cindex e4220e9..72178d9 100644 --- a/src/parallels/parallels_utils.c +++ b/src/parallels/parallels_utils.c @@ -30,6 +30,8 @@ #include "parallels_driver.h" +#define VIR_FROM_THIS VIR_FROM_PARALLELS + static int parallelsDoCmdRun(char **outbuf, const char *binary, va_list list) { @@ -105,3 +107,25 @@ parallelsCmdRun(const char *binary, ...) return ret; } + +/* + * Return new file path in malloced string created by + * concatenating first and second function arguments. + */ +char * +parallelsAddFileExt(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) { + virReportOOMError(); + return NULL; + } + + if (!virStrcpy(new_path, path, len)) + return NULL; + strcat(new_path, ext); + + return new_path; +}Peter
-- Dmitry Guryanov -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
