[libvirt] [PATCH v8 7/8] parallels: add storage driver
Dmitry Guryanov
dguryanov at parallels.com
Thu Jul 26 17:55:42 UTC 2012
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 <dguryanov at 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.c
>> index 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
More information about the libvir-list
mailing list