[libvirt] [PATCH 4/9 v2] Implement translateDiskSourcePool
Osier Yang
jyang at redhat.com
Mon Feb 4 14:22:32 UTC 2013
On 2013年02月04日 22:03, John Ferlan wrote:
> On 02/01/2013 12:24 PM, Osier Yang wrote:
>> It iterates over all the domain disks, and translate the source of
>> all the disks of 'volume' type from storage pool/volume to the real
>> underlying source.
>>
>> Disks of type 'file', 'block', and 'dir' are supported now. Network
>> type is not supported yet, it will be another patch.
>>
>> src/storage/storage_driver.c:
>> * New helper storagePoolObjFindByDiskSource to find the specified
>> pool by name.
>> * Implement translateDiskSourcePool as storageTranslateDomainDiskSourcePool
>> ---
>> src/storage/storage_driver.c | 106 ++++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 106 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
>> index e98c18c..a2c3a1a 100644
>> --- a/src/storage/storage_driver.c
>> +++ b/src/storage/storage_driver.c
>> @@ -47,6 +47,8 @@
>> #include "virlog.h"
>> #include "virfile.h"
>> #include "fdstream.h"
>> +#include "domain_conf.h"
>> +#include "domain_storage.h"
>> #include "configmake.h"
>>
>> #define VIR_FROM_THIS VIR_FROM_STORAGE
>> @@ -2367,6 +2369,104 @@ storageListAllPools(virConnectPtr conn,
>> return ret;
>> }
>>
>> +static virStoragePoolObjPtr
>> +storagePoolObjFindByDiskSource(virConnectPtr conn,
>> + const char *name)
>> +{
>> + virStorageDriverStatePtr driver = conn->storagePrivateData;
>> + virStoragePoolObjPtr pool = NULL;
>> +
>> + storageDriverLock(driver);
>> + pool = virStoragePoolObjFindByName(&driver->pools, name);
>> + storageDriverUnlock(driver);
>> +
>> + if (!pool) {
>> + virReportError(VIR_ERR_NO_STORAGE_POOL,
>> + _("no storage pool with matching name '%s'"),
>> + name);
>> + goto error;
>> + }
>> +
>> + if (!virStoragePoolObjIsActive(pool)) {
>> + virReportError(VIR_ERR_OPERATION_INVALID,
>> + _("The specified pool '%s' is not active"),
>> + name);
>> + goto error;
>> + }
>> +
>> + return pool;
>> +error:
>> + if (pool)
>> + virStoragePoolObjUnlock(pool);
>> + return NULL;
>> +}
>> +
>> +static int
>> +storageTranslateDomainDiskSourcePool(virConnectPtr conn,
>> + virDomainDefPtr def)
>> +{
>> + virStoragePoolObjPtr pool = NULL;
>> + virStorageVolDefPtr vol = NULL;
>> + int i;
>> + int ret = -1;
>> +
>> + for (i = 0; i< def->ndisks; i++) {
>> + virDomainDiskDefPtr disk = def->disks[i];
>> +
>> + if (disk->type != VIR_DOMAIN_DISK_TYPE_VOLUME)
>> + continue;
>> +
>> + if (!(pool = storagePoolObjFindByDiskSource(conn, disk->srcpool->pool)))
>> + goto cleanup;
>> +
>> + if (!(vol = virStorageVolDefFindByName(pool, disk->srcpool->volume))) {
>> + virReportError(VIR_ERR_NO_STORAGE_VOL,
>> + _("no storage volume of pool '%s' with "
>> + "matching name '%s'"),
>> + disk->srcpool->pool,
>> + disk->srcpool->volume);
>> + goto cleanup;
>> + }
>> +
>> + switch (vol->type) {
>> + case VIR_STORAGE_VOL_FILE:
>> + case VIR_STORAGE_VOL_BLOCK:
>> + case VIR_STORAGE_VOL_DIR:
>> + if (!vol->target.path&&
>> + disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM&&
>> + disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("target path of the specified volume '%s' "
>> + "(pool = '%s') is empty"),
>> + disk->srcpool->volume
>> + disk->srcpool->pool);
>> + goto cleanup;
>> + }
>> +
>> + if (vol->target.path&&
>> + !(disk->src = strdup(vol->target.path))) {
>> + virReportOOMError();
>> + goto cleanup;
>> + }
>
> Just checking - if !vol->target.path, then disk->src will be empty, is
> that expected at this point? You've added the first check since the
> initial pass at this and I want to make sure you've covered your cases.
> Without comments in the code I'm not sure what is expected.
CD-ROM and Floppy disk allow the disk source be empty. So as long
as the code flows to here. It means the disk is either a CD-ROM
or a Floppy, as we checked it before. But I agreed with you that
adding a comment here will be better. Will add it when pushing.
>
>> + break;
>> + case VIR_STORAGE_VOL_NETWORK:
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> + _("Using network volume as disk source is not supported"));
>> + goto cleanup;
>> + }
>> +
>> + virStoragePoolObjUnlock(pool);
>> + pool = NULL;
>> + }
>> +
>> + ret = 0;
>> +
>> +cleanup:
>> + if (pool)
>> + virStoragePoolObjUnlock(pool);
>> + return ret;
>> +}
>> +
>> static virStorageDriver storageDriver = {
>> .name = "storage",
>> .open = storageOpen, /* 0.4.0 */
>> @@ -2423,8 +2523,14 @@ static virStateDriver stateDriver = {
>> .reload = storageDriverReload,
>> };
>>
>> +static virDomainStorageDriver domainStorageDriver = {
>> + .translateDiskSourcePool = storageTranslateDomainDiskSourcePool,
>> +};
>> +
>> +
>> int storageRegister(void) {
>> virRegisterStorageDriver(&storageDriver);
>> virRegisterStateDriver(&stateDriver);
>> + virRegisterDomainStorageDriver(&domainStorageDriver);
>> return 0;
>> }
>>
>
> ACK (as long as you feel you've covered your cases for the disk storage
> pool)
>
> John
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
More information about the libvir-list
mailing list