Merge lp:~nuclearbob/utah/no-rewrite into lp:utah

[Max Brustkern]

This adds a config option and a command line option to remove the configuration rewriting currently done to allow an automated install.

[Javier Collado (javier.collado)]

Thanks for this change. The rewrite options is a nice step forward to be able
to manage live session scenarios.

Please find below some comments:
- CustomIntallationMixin._setuppreseed
  - This method is getting quite complex. Could it be refactored into smaller methods?
  - I think it's confusing to have both `preseed` and `self.preseed`. What
    about something like `input_preseed` and `output_preseed`?
  - The files above are opened, but the don't seem to be closed. Maybe using
    context managers will make easier to handle them?
  - Once `preseed` is properly closed, I don't think `preseed.flush` is needed

- CustomIntallationMixin._preseedcasper
  - There's a duplicated `logger.info` line
  - I see this should make possible to boot the VM from a livecd. However, it
    doesn't make possible to add scripts to automate the livecd behaviour right?
    For example, in the ISO testing tests, I see that a script is used to
    create a desktop file to be automatically executed and run some tests.

- utah.provsioning.vm.libvirtvm
  - It seems that when rewrite='minimal' just a small change is applied and
    because of that all the code for 'all' is indented one more level. I
    believe this should be refactored:

           if self.rewrite in ['all', 'minimal']:
              self.logger.debug('Setting VM to shutdown on reboot')
              xmlt.find('on_reboot').text = 'destroy'
              if self.rewrite == 'all':
                 ....
                 ....

Regarding documentation strings, what about having a table to make easier to
understand what each rewrite options means? In particular, looking at the
docstring in utah.provisioning.provisioning.Machine what I read would be
roughly equivalent to this:

          | preseed | casper | VM xml | other |
-----------------------------------------------
all | X | X | X | X |
minimal | X | X | X | |
casperonly| | X | | |
none | | | | |

However, I'm sure you can provide a better table.

review: Needs Fixing

[Javier Collado (javier.collado)]

I tried to test the changes running the following command:

$ PYTHONPATH=. ./examples/run_utah_tests.py -i <image> -p <preseed> <runlist> -d -n --rewrite casperonly

and I got this error:

Traceback (most recent call last):
  File "./examples/run_utah_tests.py", line 113, in <module>
    run_utah_tests()
  File "./examples/run_utah_tests.py", line 99, in run_utah_tests
    function(args=args)
  File "/home/javi/code/bzr/utah/no-rewrite/examples/run_install_test.py", line 91, in run_install_test
    exitstatus, locallogs = run_tests(args, machine)
  File "/home/javi/code/bzr/utah/no-rewrite/utah/run.py", line 32, in run_tests
    machine.installclient()
  File "/home/javi/code/bzr/utah/no-rewrite/utah/provisioning/provisioning.py", line 279, in installclient
    self.uploadfiles([clientdeb], tmppath)
  File "/home/javi/code/bzr/utah/no-rewrite/utah/provisioning/provisioning.py", line 532, in uploadfiles
    self.activecheck()
  File "/home/javi/code/bzr/utah/no-rewrite/utah/provisioning/vm/libvirtvm.py", line 93, in activecheck
    self.provisioncheck()
  File "/home/javi/code/bzr/utah/no-rewrite/utah/provisioning/provisioning.py", line 234, in provisioncheck
    utah.timeout.timeout(config.installtimeout, self._provision)
  File "/home/javi/code/bzr/utah/no-rewrite/utah/timeout.py", line 47, in timeout
    retval = command(*args, **kw)
  File "/home/javi/code/bzr/utah/no-rewrite/utah/provisioning/vm/libvirtvm.py", line 66, in _provision
    self._create()
  File "/home/javi/code/bzr/utah/no-rewrite/utah/provisioning/vm/libvirtvm.py", line 674, in _create
    self._installvm(lv=self.lv, tmpdir=tmpdir, xml=xml)
  File "/home/javi/code/bzr/utah/no-rewrite/utah/provisioning/vm/libvirtvm.py", line 597, in _installvm
    vm.create()
  File "/usr/lib/python2.7/dist-packages/libvirt.py", line 551, in create
    if ret == -1: raise libvirtError ('virDomainCreate() failed', dom=self)
libvirt.libvirtError: internal error cannot load AppArmor profile 'libvirt-00000000-0000-0000-0000-000000000000'

[Max Brustkern (nuclearbob)]

Refactoring the preseed setup is possible. It's setup the way it is right now so that we don't have to make multiple passes over the file. Probably there's some way to do templating or filtering that would make the whole thing less of a mess, but I'm not familiar enough with any better system to use it. If you have something in mind, I'd be happy to rewrite it using that.

The flush is currently necessary because the preseed file can be longer than the default file buffer. We flush after every line to make sure the buffer doesn't fill up. Maybe just using a larger buffer would be better.

Automating livecd behavior seems like something we'd want a custom InitRD for. The ISO testing code I think did support packing arbitrary files into an InitRD, so if we need that added to utah, that's doable. I'm not sure of the best format for that? We could support a directory structure or an archive of files to be overlaid into the initrd, or if we have a specific purpose in mind, like casper hooks, we can add something to write scripts for that. Perhaps I need to take a better look at what we're trying to do.

I suppose one more test of self.rewrite in cases where it isn't 'all' isn't going to ruin performance for everything else. I can unindent that block.

Thanks for all the feedback!

I like your table. Maybe I can make a better one, but I'm not entirely sure. I'm not great at tables.

'internal error cannot load AppArmor profile' is something I've seen before, but never consistently enough to track it down. If you're always getting that with these options, we probably need to modify the default XML.

[Javier Collado (javier.collado)]

Regarding the preseed, I'm working on some changes to parse the preseed file and expose some methods to update the contents easily and write them to another file. I'll create a merge proposal when I'm done so that you can take a look at it and let me know what do you think.

[Javier Collado (javier.collado)]

I've pushed a branch with the changes explained in the comment above here:
lp:~javier.collado/utah/preseed-parser

I haven't created a merge proposal because I don't really see huge improvements since the final code is really close to the original one (just the "for line in file" loop has been removed). Anyway, if you want to take a look at it just in case it leads to a better idea. Maybe using templates as you mentioned, could be an improvement. I've got to think about that.

[Max Brustkern (nuclearbob)]

I like your changes. The whole "line in preseed" thing seems much more like good python than just looping over the file. That also makes it very easy to say "if line in preseed, then edit the line, else add the line" which is something I've wanted to add at some point.

[Javier Collado (javier.collado)]

@Max

Please let me know if there's some way I can help with this. If you like, I could create a new branch based on yours, go through the issues in my first comment and create a merge request with fixes for some of them (I still don't know how would I boot up from a livecd though).

[Max Brustkern (nuclearbob)]

If you want to make a branch with the changes, go ahead. I've been planning to, but I've been mostly trying to get arm working first, so I had planned to come back to this next week.

=== modified file 'examples/run_install_test.py'
--- examples/run_install_test.py	2012-08-20 20:56:16 +0000
+++ examples/run_install_test.py	2012-08-30 20:00:24 +0000
@@ -61,6 +61,9 @@
                         help='Enable json logging (Default is YAML)')
     parser.add_argument('--diskbus', choices=('virtio', 'sata', 'ide'),
                         help='Disk bus to use for customvm installation')
+    parser.add_argument('--rewrite', choices=('all', 'minimal', 'casperonly',
+                        'none'), help='Enable or disable automatic '
+                        'configuration rewriting')
     return parser
 
 
@@ -84,7 +87,7 @@
                 dlpercentincrement=10, emulator=args.emulator,
                 image=args.image, initrd=args.initrd, installtype=args.type,
                 kernel=args.kernel, new=True, preseed=args.preseed,
-                series=args.series, xml=args.xml)
+                rewrite=args.rewrite, series=args.series, xml=args.xml)
         exitstatus, locallogs = run_tests(args, machine)
 
     except UTAHException as error:

=== modified file 'examples/run_test_cobbler.py'
--- examples/run_test_cobbler.py	2012-08-28 15:35:55 +0000
+++ examples/run_test_cobbler.py	2012-08-30 20:00:24 +0000
@@ -40,6 +40,9 @@
                         help='Enable debug logging')
     parser.add_argument('-j', '--json', action='store_true',
                         help='Enable json logging (Default is YAML)')
+    parser.add_argument('--rewrite', choices=('all', 'minimal', 'casperonly',
+                        'none'), help='Enable or disable automatic '
+                        'configuration rewriting')
     return parser
 
 
@@ -65,7 +68,7 @@
     try:
         inventory = ManualCobblerSQLiteInventory()
         kw = {}
-        for arg in ['image', 'preseed']:
+        for arg in ['image', 'preseed', 'rewrite']:
             value = vars(args)[arg]
             if value is not None:
                 kw.update([(arg, value)])

=== modified file 'examples/run_utah_tests.py'
--- examples/run_utah_tests.py	2012-08-14 15:42:17 +0000
+++ examples/run_utah_tests.py	2012-08-30 20:00:24 +0000
@@ -63,6 +63,9 @@
     parser.add_argument('--name', help='Name of machine to provision'
                         + ' (currently only supported for physical machine'
                         + ' provisioning)')
+    parser.add_argument('--rewrite', choices=('all', 'minimal', 'casperonly',
+                        'none'), help='Enable or disable automatic '
+                        'configuration rewriting')
     return parser
 
 

=== modified file 'utah/config.py'
--- utah/config.py	2012-08-22 15:20:25 +0000
+++ utah/config.py	2012-08-30 20:00:24 +0000
@@ -45,6 +45,7 @@
     powertimeout=5,
     preseed=None,
     qemupath='qemu:///system',
+    rewrite='all',
     series='precise',
     template=None,
 )

=== modified file 'utah/provisioning/provisioning.py'
--- utah/provisioning/provisioning.py	2012-08-29 18:46:24 +0000
+++ utah/provisioning/provisioning.py	2012-08-30 20:00:24 +0000
@@ -43,8 +43,8 @@
     def __init__(self, arch=None, debug=False, directory=None, image=None,
                  dlpercentincrement=1, initrd=None, installtype=None,
                  kernel=None, machineid=None, machineuuid=None, name=None,
-                 new=False, prefix='utah', preseed=None, series=None,
-                 template=None, xml=None):
+                 new=False, prefix='utah', preseed=None, rewrite=None,
+                 series=None, template=None, xml=None):
         """
         Initialize the object representing the machine.
         One of these groups of arguments should be included:
@@ -71,6 +71,13 @@
             new: Request a new machine (or a reinstall if a specific machine
                 was requested.)
             prefix: prefix for automatically named machines.
+            rewrite: How much to alter supplied preseed and xml files.
+                all:         everything we need for an automated install
+                minimal:     insert latecommand into preseed
+                             insert preseed into casper (desktop only)
+                             edit VM XML to handle reboots properly
+                casperonly:  insert preseed into casper (desktop only)
+                none:        do not alter preseed or VM XML
         """
         # TODO: Make this work right with super at some point.
         self.arch = arch
@@ -86,6 +93,11 @@
         self._namesetup(name)
         self._dirsetup(directory)
 
+        if rewrite is None:
+            self.rewrite = config.rewrite
+        else:
+            self.rewrite = rewrite
+
         if machineuuid is None:
             self.uuid = str(uuid.uuid4())
         else:
@@ -648,7 +660,6 @@
     Provide routines for unpacking necessary boot files from images,
     rewriting them to our liking, and repacking them.
     """
-    # TODO: setup variable(s) to suppress adding and rewriting
     def _preparekernel(self, kernel=None, tmpdir=None):
         """
         If we haven't been given a kernel file, unpack one from the image.
@@ -772,86 +783,103 @@
         self.logger.info('Setting up preseed')
         if tmpdir is None:
             tmpdir = self.tmpdir
-        preseed = open(os.path.join(tmpdir, 'initrd.d', 'preseed.cfg'), 'w')
-        for line in open(self.preseed, 'r'):
-            if 'preseed/late_command string' in line:
-                if self.installtype == 'desktop':
-                    self.logger.info('Changing d-i latecommand '
-                            + 'to ubiquity success_command '
-                            + 'and prepending ubiquity lines')
-                    line = line.replace('d-i', 'ubiquity')
-                    line = line.replace('preseed/late_command string ',
-                            ('ubiquity/success_command '
-                             'string chroot /target sh -c \''  # chroot start
-                             'export LOG_FILE=/var/log/utah-install; '
-                             'apt-get install -y --force-yes openssh-server '
-                             '>>$LOG_FILE 2>&1 ; '
-                             'apt-get install -y --force-yes gdebi-core '
-                             '>>$LOG_FILE 2>&1 ; '
-                             '\'; '),                          # chroot end
-                            1)
-                    line = ("ubiquity ubiquity/summary note\n"
-                            "ubiquity ubiquity/reboot boolean true\n"
-                            + line)
-                else:
-                    self.logger.info('Prepending latecommand')
-                    line = line.replace('preseed/late_command string ',
-                            ('preseed/late_command string '
+        if self.rewrite in ['all', 'minimal']:
+            preseed = open(os.path.join(tmpdir, 'initrd.d', 'preseed.cfg')
+                           , 'w')
+            for line in open(self.preseed, 'r'):
+                if 'preseed/late_command string' in line:
+                    if self.installtype == 'desktop':
+                        self.logger.info('Changing d-i latecommand '
+                                + 'to ubiquity success_command '
+                                + 'and prepending ubiquity lines')
+                        line = line.replace('d-i', 'ubiquity')
+                        line = line.replace('preseed/late_command string ',
+                                ('ubiquity/success_command '
+                                 'string chroot /target sh -c \''  # chroot
+                                 'export LOG_FILE=/var/log/utah-install; '
+                                 'apt-get install -y --force-yes openssh-server '
+                                 '>>$LOG_FILE 2>&1 ; '
+                                 'apt-get install -y --force-yes gdebi-core '
+                                 '>>$LOG_FILE 2>&1 ; '
+                                 '\'; '),                         # chroot end
+                                 1)
+                        line = ("ubiquity ubiquity/summary note\n"
+                                "ubiquity ubiquity/reboot boolean true\n"
+                                + line)
+                    else:
+                        self.logger.info('Prepending latecommand')
+                        line = line.replace('preseed/late_command string ',
+                                ('preseed/late_command string '
+                                 'sh utah-latecommand ; '), 1)
+                    line = line.rstrip(' ;')
+                if 'ubiquity/success_command string' in line:
+                    self.logger.info('Prepending success_command')
+                    line = line.replace('ubiquity/success_command string',
+                            ('ubiquity/success_command string '
                              'sh utah-latecommand ; '), 1)
-                line = line.rstrip(' ;')
-            if 'ubiquity/success_command string' in line:
-                self.logger.info('Prepending success_command')
-                line = line.replace('ubiquity/success_command string',
-                        ('ubiquity/success_command string '
-                         'sh utah-latecommand ; '), 1)
-                line = line.rstrip(' ;')
-            if 'pkgsel/include' in line:
-                for pkgname in ('openssh-server', 'gdebi-core'):
-                    if pkgname not in line:
-                        self.logger.info('Adding {} to preseeded packages'
-                                         .format(pkgname))
-                        line = line.replace('string ',
-                                            'string {} '.format(pkgname))
-                        line = line.rstrip() + "\n"
-            if 'netcfg/get_hostname' in line:
-                self.logger.info('Rewriting hostname to ' + self.name)
-                line = 'd-i netcfg/get_hostname string ' + self.name + "\n"
-            if 'passwd/username' in line:
-                self.logger.info('Rewriting username to ' + config.user)
-                line = 'd-i passwd/username string ' + config.user + "\n"
-            #Now that debug keeps the directory, this may not be needed
-            #self.logger.debug('Preseed line: ' + line)
-            preseed.write(line)
-            preseed.flush()
-
-        if self.installtype == 'desktop':
-            self.logger.info('Inserting preseed into casper')
-            preseedscript = os.path.join(tmpdir, 'initrd.d', 'scripts',
-                                         'casper-bottom', 'utah')
-            open(preseedscript, 'w').write(
+                    line = line.rstrip(' ;')
+                if self.rewrite == 'all':
+                    if 'pkgsel/include' in line:
+                        for pkgname in ('openssh-server', 'gdebi-core'):
+                            if pkgname not in line:
+                                self.logger.info('Adding {} to preseeded '
+                                                 'packages'.format(pkgname))
+                                line = line.replace('string ',
+                                        'string {} '.format(pkgname))
+                                line = line.rstrip() + "\n"
+                    if 'netcfg/get_hostname' in line:
+                        self.logger.info('Rewriting hostname to ' + self.name)
+                        line = ('d-i netcfg/get_hostname string ' +
+                                self.name + "\n")
+                    if 'passwd/username' in line:
+                        self.logger.info('Rewriting username to ' + config.user)
+                        line = 'd-i passwd/username string ' + config.user + "\n"
+                #Now that debug keeps the directory, this may not be needed
+                #self.logger.debug('Preseed line: ' + line)
+                preseed.write(line)
+                preseed.flush()
+        else:
+            self.logger.info('Not altering preseed because rewrite is ' +
+                             self.rewrite)
+
+        if (self.installtype == 'desktop' and
+            self.rewrite in ['all', 'minimal', 'casperonly']):
+            self._preseedcasper(tmpdir=tmpdir)
+
+    def _preseedcasper(self, tmpdir=None):
+        """
+        Insert a preseed file into casper.
+        """
+        self.logger.info('Inserting preseed into casper')
+        if tmpdir is None:
+            tmpdir = self.tmpdir
+        self.logger.info('Inserting preseed into casper')
+        preseedscript = os.path.join(tmpdir, 'initrd.d', 'scripts',
+                                     'casper-bottom', 'utah')
+        open(preseedscript, 'w').write(
 """#!/bin/sh
 
 cp preseed.cfg utah-ssh-key utah-latecommand /root
 cp 50-utahdefault.conf /root/etc/rsyslog.d
 """)
-            os.chmod(preseedscript, 0755)
-            casper_dir = os.path.join(tmpdir, 'initrd.d', 'scripts',
-                                       'casper-bottom')
-            tmpfilename = os.path.join(casper_dir, 'ORDER.tmp')
-            tmpfile = open(tmpfilename, 'w')
-            orderfilename = os.path.join(casper_dir, 'ORDER')
-            orderfile = open(orderfilename, 'r')
-            tmpfile.write(
+        os.chmod(preseedscript, 0755)
+        casper_dir = os.path.join(tmpdir, 'initrd.d', 'scripts',
+                                  'casper-bottom')
+        tmpfilename = os.path.join(casper_dir, 'ORDER.tmp')
+        tmpfile = open(tmpfilename, 'w')
+        orderfilename = os.path.join(casper_dir, 'ORDER')
+        orderfile = open(orderfilename, 'r')
+        tmpfile.write(
 """/scripts/casper-bottom/utah
 [ -e /conf/param.conf ] && . /conf/param.conf
 """)
+        tmpfile.flush()
+        for line in orderfile:
+            tmpfile.write(line)
             tmpfile.flush()
-            for line in orderfile:
-                tmpfile.write(line)
-                tmpfile.flush()
-            tmpfile.close()
-            orderfile.close()
-            os.rename(tmpfilename, orderfilename)
+        tmpfile.close()
+        orderfile.close()
+        os.rename(tmpfilename, orderfilename)
 
     def _setuplogging(self, tmpdir=None):
         """
@@ -925,34 +953,42 @@
         else:
             self.cmdline = boot
         self.boottimeout = config.boottimeout
-        if self.installtype == 'desktop':
-            if 'automatic-ubiquity' not in self.cmdline:
-                self.logger.info('Adding automatic-ubiquity option '
-                                 'since installtype is desktop')
-                self.cmdline += ' automatic-ubiquity'
-            if 'boot=' not in self.cmdline:
-                self.logger.info('Adding boot=casper option '
-                                 'since installtype is desktop')
-                self.cmdline += ' boot=casper'
-            if 'keyboard-configuration/layoutcode=' not in self.cmdline:
-                self.logger.info('Adding keyboard-configuration/layoutcode=us '
-                                 'option since installtype is desktop')
-                self.cmdline += ' keyboard-configuration/layoutcode=us'
-            if 'noprompt' not in self.cmdline:
-                self.logger.info('Adding noprompt option '
-                                 'since installtype is desktop')
-                self.cmdline += ' noprompt'
+        if self.rewrite == 'all':
+            self.logger.info('Adding needed command line options')
+            if self.installtype == 'desktop':
+                if 'automatic-ubiquity' not in self.cmdline:
+                    self.logger.info('Adding automatic-ubiquity option '
+                                     'since installtype is desktop')
+                    self.cmdline += ' automatic-ubiquity'
+                if 'boot=' not in self.cmdline:
+                    self.logger.info('Adding boot=casper option '
+                                     'since installtype is desktop')
+                    self.cmdline += ' boot=casper'
+                if 'keyboard-configuration/layoutcode=' not in self.cmdline:
+                    self.logger.info('Adding keyboard-configuration/'
+                                     'layoutcode=us option since installtype '
+                                     'is desktop')
+                    self.cmdline += ' keyboard-configuration/layoutcode=us'
+                if 'noprompt' not in self.cmdline:
+                    self.logger.info('Adding noprompt option '
+                                     'since installtype is desktop')
+                    self.cmdline += ' noprompt'
+            else:
+                if (self.installtype == 'server'
+                    and 'DEBCONF_DEBUG' not in self.cmdline):
+                    self.logger.info('Adding DEBCONF_DEBUG=developer option '
+                                     'since installtype is not server')
+                    self.cmdline += ' DEBCONF_DEBUG=developer'
+                if 'debconf/priority=' not in self.cmdline:
+                    self.logger.info('Adding debconf/priority=critical option '
+                                     'since installtype is not desktop')
+                    self.cmdline += ' debconf/priority=critical'
         else:
-            if (self.installtype == 'server'
-                and 'DEBCONF_DEBUG' not in self.cmdline):
-                self.logger.info('Adding DEBCONF_DEBUG=developer option '
-                                 'since installtype is not server')
-                self.cmdline += ' DEBCONF_DEBUG=developer'
-            if 'debconf/priority=' not in self.cmdline:
-                self.logger.info('Adding debconf/priority=critical option '
-                                 'since installtype is not desktop')
-                self.cmdline += ' debconf/priority=critical'
+            self.logger.info('Not altering command line since rewrite is ' +
+                             self.rewrite)
         self.cmdline.strip()
+        self.logger.debug('Boot command line is:')
+        self.logger.debug(self.cmdline)
 
     def _custominit(self, arch=None, boot=None, installtype=None, series=None):
         """

=== modified file 'utah/provisioning/vm/libvirtvm.py'
--- utah/provisioning/vm/libvirtvm.py	2012-08-24 18:26:16 +0000
+++ utah/provisioning/vm/libvirtvm.py	2012-08-30 20:00:24 +0000
@@ -477,97 +477,104 @@
         if tmpdir is None:
             tmpdir = self.tmpdir
         xmlt = ElementTree.ElementTree(file=xml)
-        self.logger.debug('Rewriting basic info')
-        xmlt.find('name').text = self.name
-        xmlt.find('uuid').text = self.uuid
-        self.logger.debug('Setting type to qemu '
-                          'in case no hardware virtualization present')
-        xmlt.getroot().set('type', self.domaintype)
-        ose = xmlt.find('os')
-        if self.arch == ('i386'):
-            ose.find('type').set('arch', 'i686')
-        elif self.arch == ('amd64'):
-            ose.find('type').set('arch', 'x86_64')
-        else:
-            ose.find('type').set('arch', self.arch)
-        self.logger.debug('Setting up boot info')
-        for kernele in list(ose.iterfind('kernel')):
-            ose.remove(kernel)
-        kernele = ElementTree.Element('kernel')
-        kernele.text = kernel
-        ose.append(kernele)
-        for initrde in list(ose.iterfind('initrd')):
-            ose.remove(initrd)
-        initrde = ElementTree.Element('initrd')
-        initrde.text = initrd
-        ose.append(initrde)
-        for cmdline in list(ose.iterfind('cmdline')):
-            ose.remove(cmdline)
-        cmdlinee = ElementTree.Element('cmdline')
-        cmdlinee.text = cmdline
-        ose.append(cmdlinee)
-        xmlt.find('on_reboot').text = 'destroy'
-        self.logger.debug('Setting up devices')
-        devices = xmlt.find('devices')
-        self.logger.debug('Setting up disks')
-        for disk in list(devices.iterfind('disk')):
-            if disk.get('device') == 'disk':
-                devices.remove(disk)
-                self.logger.debug('Removed existing disk')
-            #TODO: Add a cdrom if none exists
-            if disk.get('device') == 'cdrom':
-                if disk.find('source') is not None:
-                    disk.find('source').set('file', self.image.image)
-                    self.logger.debug('Rewrote existing CD-ROM')
+        if self.rewrite in ['all', 'minimal']:
+            self.logger.debug('Setting VM to shutdown on reboot')
+            xmlt.find('on_reboot').text = 'destroy'
+            if self.rewrite == 'all':
+                self.logger.debug('Rewriting basic info')
+                xmlt.find('name').text = self.name
+                xmlt.find('uuid').text = self.uuid
+                self.logger.debug('Setting type to qemu in case no '
+                                  'hardware virtualization present')
+                xmlt.getroot().set('type', self.domaintype)
+                ose = xmlt.find('os')
+                if self.arch == ('i386'):
+                    ose.find('type').set('arch', 'i686')
+                elif self.arch == ('amd64'):
+                    ose.find('type').set('arch', 'x86_64')
                 else:
+                    ose.find('type').set('arch', self.arch)
+                self.logger.debug('Setting up boot info')
+                for kernele in list(ose.iterfind('kernel')):
+                    ose.remove(kernel)
+                kernele = ElementTree.Element('kernel')
+                kernele.text = kernel
+                ose.append(kernele)
+                for initrde in list(ose.iterfind('initrd')):
+                    ose.remove(initrd)
+                initrde = ElementTree.Element('initrd')
+                initrde.text = initrd
+                ose.append(initrde)
+                for cmdline in list(ose.iterfind('cmdline')):
+                    ose.remove(cmdline)
+                cmdlinee = ElementTree.Element('cmdline')
+                cmdlinee.text = cmdline
+                ose.append(cmdlinee)
+                self.logger.debug('Setting up devices')
+                devices = xmlt.find('devices')
+                self.logger.debug('Setting up disks')
+                for disk in list(devices.iterfind('disk')):
+                    if disk.get('device') == 'disk':
+                        devices.remove(disk)
+                        self.logger.debug('Removed existing disk')
+                    #TODO: Add a cdrom if none exists
+                    if disk.get('device') == 'cdrom':
+                        if disk.find('source') is not None:
+                            disk.find('source').set('file', self.image.image)
+                            self.logger.debug('Rewrote existing CD-ROM')
+                        else:
+                            source = ElementTree.Element('source')
+                            source.set('file', self.image.image)
+                            disk.append(source)
+                            self.logger.debug('Added source to existing '
+                                              'CD-ROM')
+                for disk in self.disks:
+                    diske = ElementTree.Element('disk')
+                    diske.set('type', 'file')
+                    diske.set('device', 'disk')
+                    driver = ElementTree.Element('driver')
+                    driver.set('name', 'qemu')
+                    driver.set('type', disk['type'])
+                    diske.append(driver)
                     source = ElementTree.Element('source')
-                    source.set('file', self.image.image)
-                    disk.append(source)
-                    self.logger.debug('Added source to existing CD-ROM')
-        for disk in self.disks:
-            diske = ElementTree.Element('disk')
-            diske.set('type', 'file')
-            diske.set('device', 'disk')
-            driver = ElementTree.Element('driver')
-            driver.set('name', 'qemu')
-            driver.set('type', disk['type'])
-            diske.append(driver)
-            source = ElementTree.Element('source')
-            source.set('file', disk['file'])
-            diske.append(source)
-            target = ElementTree.Element('target')
-            dev = "vd%s" % (string.ascii_lowercase[self.disks.index(disk)])
-            target.set('dev', dev)
-            target.set('bus', disk['bus'])
-            diske.append(target)
-            devices.append(diske)
-            self.logger.debug('Added ' + str(disk['size']) + ' disk')
-        macs = list(self.macs)
-        for interface in devices.iterfind('interface'):
-            if interface.get('type') in ['network', 'bridge']:
-                if len(macs) > 0:
-                    mac = macs.pop(0)
-                    interface.find('mac').set('address', mac)
-                    self.logger.debug('Rewrote interface '
-                                      'to use specified mac address ' + mac)
-                else:
-                    mac = random_mac_address()
-                    interface.find('mac').set('address', mac)
-                    self.macs.append(mac)
-                    self.logger.debug('Rewrote interface '
-                                      'to use random mac address ' + mac)
-            if interface.get('type') == 'bridge':
-                interface.find('source').set('bridge', config.bridge)
-        serial = ElementTree.Element('serial')
-        serial.set('type', 'file')
-        source = ElementTree.Element('source')
-        log_filename = os.path.join(config.logpath, self.name + '.syslog.log')
-        source.set('path', log_filename)
-        serial.append(source)
-        target = ElementTree.Element('target')
-        target.set('port', '0')
-        serial.append(target)
-        devices.append(serial)
+                    source.set('file', disk['file'])
+                    diske.append(source)
+                    target = ElementTree.Element('target')
+                    dev = "vd%s" % (string.ascii_lowercase[self.disks.index(disk)])
+                    target.set('dev', dev)
+                    target.set('bus', disk['bus'])
+                    diske.append(target)
+                    devices.append(diske)
+                    self.logger.debug('Added ' + str(disk['size']) + ' disk')
+                macs = list(self.macs)
+                for interface in devices.iterfind('interface'):
+                    if interface.get('type') in ['network', 'bridge']:
+                        if len(macs) > 0:
+                            mac = macs.pop(0)
+                            interface.find('mac').set('address', mac)
+                            self.logger.debug('Rewrote interface '
+                                              'to use specified mac address ' + mac)
+                        else:
+                            mac = random_mac_address()
+                            interface.find('mac').set('address', mac)
+                            self.macs.append(mac)
+                            self.logger.debug('Rewrote interface '
+                                              'to use random mac address ' + mac)
+                    if interface.get('type') == 'bridge':
+                        interface.find('source').set('bridge', config.bridge)
+                serial = ElementTree.Element('serial')
+                serial.set('type', 'file')
+                source = ElementTree.Element('source')
+                log_filename = os.path.join(config.logpath, self.name + '.syslog.log')
+                source.set('path', log_filename)
+                serial.append(source)
+                target = ElementTree.Element('target')
+                target.set('port', '0')
+                serial.append(target)
+                devices.append(serial)
+        else:
+            self.logger.info('Not rewriting XML because rewrite is ' +
+                             self.rewrite)
         if self.debug:
             xmlt.write(os.path.join(tmpdir, 'install.xml'))
         self.logger.info('Installation XML ready')
@@ -609,16 +616,23 @@
             xml = ElementTree.ElementTree(file=self.xml)
         if tmpdir is None:
             tmpdir = self.tmpdir
-        ose = xml.find('os')
-        for kernel in ose.iterfind('kernel'):
-            ose.remove(kernel)
-        for initrd in ose.iterfind('initrd'):
-            ose.remove(initrd)
-        for cmdline in ose.iterfind('cmdline'):
-            ose.remove(cmdline)
-        xml.find('on_reboot').text = 'restart'
-        devices = xml.find('devices')
-        devices.remove(devices.find('serial'))
+        if self.rewrite in ['all', 'minimal']:
+            self.logger.debug('Setting VM to reboot normally on reboot')
+            xml.find('on_reboot').text = 'restart'
+            if self.rewrite == 'all':
+                self.logger.debug('Removing VM install parameters')
+                ose = xml.find('os')
+                for kernel in ose.iterfind('kernel'):
+                    ose.remove(kernel)
+                for initrd in ose.iterfind('initrd'):
+                    ose.remove(initrd)
+                for cmdline in ose.iterfind('cmdline'):
+                    ose.remove(cmdline)
+                devices = xml.find('devices')
+                devices.remove(devices.find('serial'))
+        else:
+            self.logger.info('Not rewriting XML because rewrite is ' +
+                             self.rewrite)
         if self.debug:
             xml.write(os.path.join(tmpdir, 'final.xml'))
         return xml
@@ -644,7 +658,11 @@
 
         self._setuppreseed(tmpdir=tmpdir)
 
-        self._setuplogging(tmpdir=tmpdir)
+        if self.rewrite == 'all':
+            self._setuplogging(tmpdir=tmpdir)
+        else:
+            self.logger.debug('Skipping logging setup because rewrite is' +
+                              self.rewrite)
 
         initrd = self._repackinitrd(tmpdir=tmpdir)