6:55 a.m.
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 :|