10:22 p.m.
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list