A couple other things, looking through this:
1) The "if self.create_symlink:" else raise InfoException part, I think, is
logically wrong. What this will do is cause an exception if someone
doesn't set the option, when you should only raise that error if the
symlink call fails.
2) The function "def create_symlink(name)" is not named correctly, people
may think this is a general purpose symlink function when it is really
creating symlinks only to /etc/xen/. As Michael pointed out, there may be
a way to do this through libvirt and that should be used, but if not this
function should be renamed so that it doesn't cause confusion.
3) Using the option to auto-start a VM should only be used when --virt is
specified, since koan does do other things too. I can't tell by your patch
if that is the case, but if not it should be corrected.
On Thu, 19 Mar 2009 06:30:17 -0500, James Cammarata <jimi(a)sngx.net> wrote:
The only thing I'd say about this is that the option is a little
too
literal: you are creating a symlink but what you're really doing is
setting
the vm to start when the server boots, so I would prefer to see the
option
named something like --auto, --auto-start, --start-on-boot, or
something
along those lines.
On Wed, 18 Mar 2009 17:40:47 -0700, Adam Rosenwald <thestrider(a)gmail.com>
wrote:
> This proposed patch for koan allows one to create a symlink from
> /etc/xen/<system> to /etc/xen/auto/<system>, a common post deployment
> task for admins.
>
> Example: koan -s <server> -y <system> ... [-x|--create-symlink]
>
> The patch is based on the latest koan git commit
> (e6db4239a5ad1596c5b9d87d5669f70694773198).
>
> =====
> app.py
> =====
> --- app.py 2009-03-18 23:59:58.000000000 +0000
> +++ /home/arosenwald/koan/koan/app.py 2009-03-18 23:44:01.000000000
> +0000
> @@ -138,6 +138,10 @@
> action="store_true",
> dest="no_gfx",
> help="disable Xen graphics (xenpv,xenfv)")
> + p.add_option("-x", "--create-symlink",
> + action="store_true",
> + dest="create_symlink",
> + help="create /etc/xen/auto symlinks (for autoboot)")
> p.add_option("", "--add-reinstall-entry",
> dest="add_reinstall_entry",
> action="store_true",
> @@ -180,6 +184,7 @@
> k.use_kexec = options.use_kexec
> k.should_poll = options.should_poll
> k.embed_kickstart = options.embed_kickstart
> + k.create_symlink = options.create_symlink
>
> if options.virt_name is not None:
> k.virt_name = options.virt_name
> @@ -1071,6 +1076,10 @@
> else:
> raise InfoException("internal error, bad virt
state")
>
> + if self.create_symlink:
> + utils.create_symlink(virtname)
> + else:
> + raise InfoException("cannot create /etc/xen/auto/%s
> symlink. please check permissions and ownership of /etc/xen/auto." %
> virtname)
> return results
>
> #---------------------------------------------------
>
> =====
> utils.py
> =====
> --- utils.py 2009-03-18 23:59:58.000000000 +0000
> +++ /home/arosenwald/koan/koan/utils.py 2009-03-18 23:59:39.000000000
> +0000
> @@ -377,6 +377,19 @@
> return server
> raise InfoException ("Could not find Cobbler.")
>
> +def create_symlink(name):
> + """
> + Create an /etc/xen/auto/<name> symlink for use with
"xendomains"-style
> + VM boot upon dom0 reboot.
> + """
> + src = "/etc/xen/%s" % name
> + dst = "/etc/xen/auto/%s" % name
> +
> + # check that xen config file exists and create symlink
> + if os.path.exists(src) and os.access(os.path.dirname(dst),
os.W_OK):
> + os.symlink(src, dst)
> + else:
> + raise InfoException("Could not create /etc/xen/auto/%s
> symlink. Please check write permissions and ownership" % name)
>
> class ServerProxy(xmlrpclib.ServerProxy):
>
> _______________________________________________
> cobbler-devel mailing list
> cobbler-devel(a)lists.fedorahosted.org
>
https://fedorahosted.org/mailman/listinfo/cobbler-devel
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.