[et-mgmt-tools] Virt-disk-path selection patch and other virt. attributes...
Michael DeHaan
mdehaan at redhat.com
Tue Jul 10 18:12:22 UTC 2007
Adam Rosenwald wrote:
> 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.
Ok, great ... I'll have to do some testing on this to see how it all
works, but I'm glad to see this.
If anyone else has comments on the virt-path handling (especially the
volume group and LVM parts), please say so -- more eyes on this would be
useful.
Basically the idea here is to allow the following attributes to cobbler
profile/system add:
--virt-path=/path/to/disk/image
--virt-path=partition:/dev/device
--virt-path=volume-group:name
This is rather useful -- Currently guests are installed exclusively to
/var/lib/xen/images -- which is a good default, but isn't
aware of external storage, SANs, and other nice shiny things.
Also profile/system add would get the option
--virt-cpus=number
and the following attributes to "cobbler system add" (not profile add)
--uiid=foo
Possibly koan should also get an override parameter for --virt-path in
the event that the admin of a server has set up a particular path
for their config, but a one-off deployment is warranted (say, for
testing) without needing the creation of yet another profile/system.
This also
raises the question of whether we want other parameters overridable in
koan, though in general I'd say we want to keep that to a minimum.
--Michael
>
> --- /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.
>>
>>
>
>
>
> _______________________________________________
> et-mgmt-tools mailing list
> et-mgmt-tools at redhat.com
> https://www.redhat.com/mailman/listinfo/et-mgmt-tools
More information about the et-mgmt-tools
mailing list