[et-mgmt-tools] Virt-disk-path selection patch and other virt. attributes...
Adam Rosenwald
thestrider at gmail.com
Mon Jul 9 23:13:27 UTC 2007
Hey Michael et al.
The following is an (unpolished) method for virt-path (and a few other
discussed virt-attribute methods) in koan's app.py. I've tested it in
parts; it appears to work. :) More extensive testing will ensue next
week. I also have a comment or two on Michael's last response (inline).
Thanks.
--- /usr/lib/python2.4/site-packages/koan/app.py.orig 2007-07-04
21:56:10.000000000 -0400
+++ /usr/lib/python2.4/site-packages/koan/app.py 2007-07-06
18:09:41.000000000 -0400
@@ -30,6 +30,8 @@
import xmlrpclib
import string
+from stat import *
+
"""
koan --virt [--profile=webserver|--system=name] --server=hostname
koan --replace-self --profile=foo --server=hostname
@@ -160,8 +162,6 @@
raise InfoException, "must use either --virt or --replace-self"
if self.is_virt and self.is_auto_kickstart:
raise InfoException, "must use either --virt or --replace-self"
- if not self.server:
- raise InfoException, "no server specified"
if self.verbose is None:
self.verbose = True
if (not self.profile and not self.system):
@@ -572,8 +572,10 @@
results = virtcreate.start_paravirt_install(
name=name,
ram=self.calc_virt_ram(pd),
- disk= self.calc_virt_filesize(pd),
+ disk=self.calc_virt_filesize(pd),
+ vcpus=self.calc_virt_cpus(pd),
mac=virtcreate.get_mac(self.calc_virt_mac(pd)),
+ path=self.set_virt_path(pd, name, mac),
uuid=virtcreate.get_uuid(self.calc_virt_uuid(pd)),
kernel=self.safe_load(pd,'kernel_local'),
initrd=self.safe_load(pd,'initrd_local'),
@@ -583,11 +585,24 @@
print results
def calc_virt_uuid(self,data):
- # TODO: eventually we may want to allow some koan CLI
- # option for passing in the UUID. Until then, it's random.
- return None
+ """
+ Assign a UUID if none/invalid is given in the profile.
+ """
+ id = self.safe_load(data,'virt_uuid','xen_uuid',0)
+ uuid_re =
re.compile('[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}')
+ err = False
+ try:
+ str(id)
+ except:
+ err = True
+ if id is None or id == '' or not uuid_re.match(id):
+ err = True
+ if err:
+ self.debug("invalid UUID specified. randomizing...")
+ return None
+ return id
- def calc_virt_filesize(self,data):
+ def calc_virt_filesize(self,data,default_filesize=1):
"""
Assign a virt filesize if none is given in the profile.
"""
@@ -597,14 +612,14 @@
int(size)
except:
err = True
+ if size is None or size == '' or int(size)<default_filesize:
err = True
if err:
- self.debug("invalid file size specified, defaulting to 1 GB")
- return 1
+ self.debug("invalid file size specified, defaulting to %s
GB" % default_filesize)
+ return default_filesize
return int(size)
- def calc_virt_ram(self,data):
+ def calc_virt_ram(self,data,default_ram=256):
"""
Assign a virt ram size if none is given in the profile.
"""
@@ -617,8 +632,26 @@
if size is None or size == '' or int(size) < 128:
err = True
if err:
- self.debug("invalid RAM size specified, defaulting to 256 MB")
- return 256
+ self.debug("invalid RAM size specified, defaulting to %s
MB" % default_ram)
+ return default_ram
+ return int(size)
+
+
+ def calc_virt_cpus(self,data,default_cpus=1):
+ """
+ Assign virtual CPUs if none is given in the profile.
+ """
+ size = self.safe_load(data,'virt_cpus','xen_cpus',0)
+ err = False
+ try:
+ int(size)
+ except:
+ err = True
+ if size is None or size == '' or int(size) < default_cpus:
+ err = True
+ if err:
+ self.debug("invalid number of VCPUS specified, defaulting
to % VCPU" % default_cpus)
+ return default_cpus
return int(size)
@@ -632,5 +665,69 @@
return self.system.upper()
return None
+ def set_virt_path(self,data,name,mac):
+ """
+ Assign virtual disk location.
+ """
+ location = self.safe_load(data,'virt_path','xen_path',0)
+ if location.find(':') == -1:
+ # LOOPBACK FILE
+ if os.path.isabs(location):
+ # Location is absolute path
+ if location.endswith("/"):
+ if name == None:
+ path="%s%s" % (location,mac)
+ else:
+ path="%s%s" % (location,name)
+ else:
+ path=location
+ else:
+ # Location is relative path
+ if location.endswith("/"):
+ if name == None:
+ path="/var/lib/xen/images/%s%s" % (location,mac)
+ else:
+ path="/var/lib/xen/images/%s%s" % (location,name)
+ else:
+ path="/var/lib/xen/images/%s" % location
+ else:
+ # PARTITION or VG
+ count = location.count(':')
+ # Dangerous since pathname may legally include ':'
+ if count == 1:
+ (type,blk_id)=location.split(':')
+ else:
+ self.debug("Please enter one, two, or three delimited
entries in your virt_path file. %s has %d entries which exceeds the
maximum" % (location, count))
+ return None
+
+ # FOR PARTITIONS
+ if type == "partition" or type == "part":
+ if os.path.exists(blk_id) and
S_ISBLK(os.stat(blk_id)[ST_MODE]):
+ # virtinst takes care of freespace checks
+ path=blk_id
+ else:
+ raise InfoException, "Either virt_path [%s] does
not exist or [%s] is not a block device." % (blk_id,blk_id)
+
+ # FOR VG/LV
+ if type == "vg" or type == "volume-group":
+ vgnames = sub_process.Popen(["vgs", "-o", "vg_name",
"--noheadings" ], stdout=sub_process.PIPE).communicate()[0]
+ if vgnames.find(blk_id) == -1:
+ raise InfoException, "The volume group [%s] does
not exist. Please respecify virt_path" % blk_id
+ # check free space
+ lv_freespace_str = sub_process.Popen(["lvs",
"--noheadings", "-o", "vg_free", "--units", "g", blk_id2],
stdout=sub_process.PIPE).communicate()[0]
+ vg_freespace = int(float(vg_freespace_str.strip()[0:-1]))
+ lv_size =
self.safe_load(data,'virt_file_size','xen_file_size',0)
+ if vg_freespace >= int(lv_size):
+ # Sufficient space
+ lvs_str=sub_process.Popen(["lvs", "--noheadings",
"-o", "lv_name", blk_id], stdout=subprocess.PIPE).communicate()[0]
+ if name == None:
+ if not lvs_str.find(mac):
+ lv_create = sub_process.Popen(["lvcreate",
"-L", "%sG" % lv_size, "-n", mac, blk_id],
stdout=sub_process.PIPE).communicate()[0]
+ path="/dev/%s/%s" % (blk_id,mac)
+ else:
+ if not lvs_str.find(name):
+ lv_create = sub_process.Popen(["lvcreate",
"-L", "%sG" % lv_size, "-n", name, blk_id],
stdout=sub_process.PIPE).communicate()[0]
+ path="/dev/%s/%s" % (blk_id,name)
+ else:
+ raise InfoException, "The volume group [%s] does
not have at least %sGB free space." % lv_size
+ return path
+
if __name__ == "__main__":
main()
Michael DeHaan wrote:
> Adam Rosenwald wrote:
>> Michael,
>>
>> Here are some starter patches for koan in preparation for a
>> proposed migration of virtual attributes to cobbler. The uuid and
>> vcpu methods can be used directly with only trivial adjustments to
>> virtcreate.py.
>
> Cool. Some comments below...
>>
>> This is just some practice for migrating koan virtual attribute
>> checking and default assignment over to cobbler profiles. I don't
>> know whether you would like this as default behavior, but I was
>> thinking that if the user specifies "path" for --virt-path (a
>> proposed CL argument for passing the location of a prespecified
>> virtual disk file), the following behavior could be taken:
>>
>> # Since block IO is superior to file IO, check to see if --virt-path
>> corresponds to block devices
>
> I like these ideas. I've revised the following has been revised a
> bit... mainly I prefer my arguments to be a bit more predictable in
> what they do, and this makes it clearer to to understand (for the
> user). It also makes documenting what to specify for --virt-path a
> lot simpler. I didn't think of the Volume Group creation idea before,
> and if reasonable simple to implement, that would be a great feature.
>
> Anyhow, for disambigution, let's have the following syntax:
>
> --virt-path=foo --virt-path=volume-group:foo
> --virt-path=partition:foo
Full disambiguation would be
--virt-path=file:foo --virt-path=vg:foo
--virt-path=partition:foo
... but I've stuck with your reasoning, letting --virt-path=foo default
to loopback files.
>
> (And also let's assume that we can override the cobbler value for
> --virt-path in koan, if needed.)
>
>> * For volume groups
>> o Does a volume_group exist named --virt_path?
>>
> Question: what if system.name already
> exists in the volume group?
Reuse it. The assumption is that the user knows what (s)he's doing.
This is already implemented as a check near the end of the set_virt_path
method
>>
>> + If so
>> # Calculate how much space is left in VG
>> "virt_path"
>> # If enough space to create LV with --virt-disk
>> size
>> * Create new LV named after system.name,
>> unless overriden with --virt-name.
>> Note that system names in 0.5.0 can be
>> arbitrary and are
>>
> not necessarily
> MAC addresses.
>>
>> * If not enough, FAIL
>>
>> * For --virt-paths
>>
> pass %PATH/%virtname into XenDisk , if not in
> /var/lib/xen/images and SELinux is enabled, adjust security context
> appropriately (chcon).
You or someone else will have to implement the SELinux context
adjustments. This is a little beyond me for the moment...
>
> For explicit partitions:
> use the LVM partition as entered, if it does not
> exist, fail.
> Example of koan value being overridden by koan:
>
> cobbler profile add --name=foo --distro=bar
> --virt-path=/mnt/storage/diskimages
> koan --virt --server=bootserver.example.com
> --virt-path=/var/lib/xen/images #override
>
>
>> -------
>>
>> Just some thoughts. The additional virt-attributes: bridge, vnc,
>> nographics, etc. should be customizable as well. The 'size'
>> attributes (vcpu,vram,vdisksize) are inheritable by child profiles.
> >> Also (and you may dispute this), the vpath (virtual disk path) is
> inheritable by the above program logic. The benefit of inheritance
> here allows systems that use the same profile to make use of the same
> VG. This way, LVM is simplified (and, errm, added) with cobbler
> profiles.
I have yet to begin migrating things over to cobbler (so that we may
experience virtual attribute inheritance!). This would require changing
core profile collection data structures and the like. I will play
around with this if you want, but given the fact that these data
structures are key to so many other functions of cobbler, I would feel
better if I didn't do this alone.
>
> Yes, all values should be inheritable unless there's a reason for them
> to not be. (Example: currently the --distro parameter on the cobbler
> profile object is not inheritable)
>>
>> In general, what is your recommended strategy for the actual
>> migration of virtual attributes from koan to cobbler? I think the
>> checks should be done on the cobbler side, so that end users don't
>> waste time creating child profiles with genetic flaws (i.e. children
>> who inherit bad attributes). It's a bit of work, but it's worth doing.
> The way cobbler currently deals with arguments can be used here with
> no changes and can still give you want you want. Storage locations
> themselves can't be totally validated server side, and should be
> accepted as little more than strings. If the value given is
> specified as a path, we can check to see that it starts with "/", but
> not much more.
>
> "It's a bit of work, but it's worth doing."
>
> Cobbler's existing validation system prevents the assigning of a value
> that doesn't pass validation, so that won't trickle down -- I don't
> think this is a concern. Can you give an example?
>
>>
>> BTW, I'm sorry for not getting back to you sooner with more patches.
>> I've been busy with job-related stuff. :)
>>
>>
> np ... given there is already quite a lot (too much) in 0.5.0, I'll
> probably wait until 0.5.0 is out to begin rolling this in.
>
> The main thing I would pay attention to with the path logic is making
> things as explicit as possible. Having storage decisions be
> unpredictable based on the
> local storage configuration would be bad IMHO ... I would rather
> things fail than store data at a location that wasn't exactly what the
> cobbler admin requested,
> or otherwise provide some way of specifying a list of criteria.
>
> Thanks!
>
> --Michael
>
>
>>
>>
>> ## BEGIN PATCH ##
>> --- app.py.orig 2007-06-21 22:09:15.000000000 +0000
>> +++ app.py 2007-06-21 23:58:09.000000000 +0000
>> @@ -630,9 +630,45 @@
>> print results
>>
>> def calc_virt_uuid(self,data):
>> - # TODO: eventually we may want to allow some koan CLI
>> - # option for passing in the UUID. Until then, it's random.
>> - return None
>> + """
>> + Assign a UUID if none/invalid is given in the profile.
>> + """
>> + id = self.safe_load(data,'virt_uuid','xen_uuid',0)
>> + uuid_re =
>> re.compile('[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}')
>>
>> + err = False
>> + try:
>> + str(id)
>> + except:
>> + err = True
>> + if id is None or id == '' or ! uuid_re.match(id):
>> + err = True
>> + if err:
>> + self.debug("invalid UUID specified. randomizing...")
>> + return None
>> + return id
>> +
>> + def calc_virt_path(self,data):
>> + """
>> + Assign a virt file path if none is given in the profile.
>> + """
>> + # Determine if path is string
>> + # Determine if path is file
>> + # Determine if path is symlink
>> + # Determine if path/symlink is block device or valid FS
>> + # VG checks/allocation, loopback file checks, or default file
>> assignment
>> +
>> +
>> + # TODO
>> +
>> + def non_sparse(self,data):
>> + """
>> + TODO -- Is nonsparse specified? If not, provide default
>> + """
>> +
>> + def additional(self,data):
>> + """
>> + TODO -- More checks, default assignments for additional virt
>> attributes
>> + """
>>
>> def calc_virt_filesize(self,data):
>> """
>> @@ -669,6 +705,23 @@
>> return int(size)
>>
>>
>> + def calc_virt_cpus(self,data):
>> + """
>> + Assign virtual CPUs if none is given in the profile.
>> + """
>> + size = self.safe_load(data,'virt_cpus','xen_cpus',0)
>> + err = False
>> + try:
>> + int(size)
>> + except:
>> + err = True
>> + if size is None or size == '' or int(size) < 1:
>> + err = True
>> + if err:
>> + self.debug("invalid number of VCPUS specified,
>> defaulting to 1 VCPU")
>> + return 1
>> + return int(size)
>> +
>> def calc_virt_mac(self,data):
>> """
>> For now, we have no preference.
>> ### END PATCH ###
>>
>>
>> --A.
>
>
More information about the et-mgmt-tools
mailing list