On Wed, Aug 17, 2011 at 12:53:00PM +0800, Lei Li wrote:
> Hi Daniel,
>
> The last version of my patch fixed bug #611823 prohibit pools with duplicate storage,
> you gave some comments that should do check by source info instead of target path I used.
> These days I view the code and existing documents about storage pool, and tried to
> code based on your comments, but looks like it is turning out to be more complex.
>
> First, I think it is possible to make clear what it means for a pool to be a duplicate.
> Below is the detailed source info be used for each type of pool according to the existing
> documents and the main item to control.
>
> For details of each type of pool:
>
> VIR_STORAGE_POOL_DIR, /* For dir type, use source->dir only(But in the code and
> example XML file,it use target path).*/
> A pool with a type of dir provides the means to manage files within a directory.
For the type=dir, the source->dir and target->path are always identical,
so they can be used interchangably in the code. The XML should generally
specify the source dir, and the target path will be automatically filled
in from this.
> VIR_STORAGE_POOL_FS, /* For fs type, use source->device->path.*/
> This is a variant of the directory pool. Instead of creating a directory on an
> existing mounted filesystem though, it expects a source block device to be named.
> This block device will be mounted and files managed in the directory of its mount point.
>
> VIR_STORAGE_POOL_NETFS, /* For netfs, use host name and source->dir */
> This is a variant of the filesystem pool. Instead of requiring a local block device as
> the source, it requires the name of a host and path of an exported directory. It will
> mount this network filesystem and manage files within the directory of its mount point.
>
> VIR_STORAGE_POOL_LOGICAL, /* For logical, use source->device->path. */
> This provides a pool based on an LVM volume group. For a pre-defined LVM volume group,
> simply providing the group name is sufficient,while to build a new group requires
> providing a list of source devices to serve as physical volumes.
>
> VIR_STORAGE_POOL_DISK, /* For disk type, use source->device->path.*/
> This provides a pool based on a physical disk. Volumes are created by adding
> partitions to the disk.
These are all correct.
> VIR_STORAGE_POOL_ISCSI, /* For ISCSI type, use source->device->path. */
> This provides a pool based on an iSCSI target.
As well as the 'device' which maps to the iSCSI target name, we also
use the hostname. There is finally also an optional initiator IQN.
If no IQN is specified, then we use the host's default. Uniqueness
checks need to be based on (hostname, device, iqn)
> VIR_STORAGE_POOL_SCSI, /* For SCSI type, use source->adapter name. */
> This provides a pool based on a SCSI HBA.
>
> VIR_STORAGE_POOL_MPATH,
> This provides a pool that contains all the multipath devices on the host.
> Volume creating is not supported via the libvirt APIs, so we will just
> ignore this type.
These are all correct.
>
> After a preliminary investigation, for source element,
>
> source->device->path,
> -Pool type: fs, logical, disk, ISCSI.
> -May only occur once.
> source->dir,
> -Pool type: dir, netfs.
> -May only occur once.
> source->adapter
> -Pool type: SCSI.
> -May only occur once.
> are the main required location info, and we can control the duplicate storage
> based on these three item above for related type of pool.
>
> Second, I have tried to do check by source info for a test, when I debug, I found
> that pools->objs[]->def->source.* which come from virConnectPtr where all existing
> pools info be keeped == NULL, but the current target pool virStoragePoolDefPtr def
> can get source field value, so I think libvirt didn't get source field info in the
> pools list where we do check at all today.
>
> Do you think we should do check based on this? Or any comments for question one and
> two. After you confirmed then I will work on code. Thank you.
Rather than trying to group things according to the source elements
used, group according to the pool type. For each pool type, check the
source information against source info for other pools of the same
type. eg
if (type == dir) {
...check source dir against other pools with type=dir...
} else if (type == netfs) {
...check (host, dir) against other pools with type=netfs...
} else if (...) {
....
} else if (type == scsi) {
...check adapter against other pools with type=scsi
}
Finally, there is one extra check which can be applied to all the
file based pools at the same time:
if (type == dir || type == netfs || type == fs) {
..check target path is unique against other pools with
type==dir||netfs||fs...
}
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list