On Mon, 2019-12-09 at 16:20 +0100, Fabiano Fidêncio wrote:
> Let's always generate the unattended script file as, by doing this, we
> can trea the files we have as templates, being able to change them
s/trea/treat/
> accordingly to whatever is needed by specific distros.
>
> It's important to note that we *must* be careful and keep generating
> those files using their "expected" filename, preferrably using the same
s/preferrably/preferably/
[...]
> @@ -537,7 +539,14 @@ class Application:
> raise Exception(
> "Host {} doesn't support installation".format(host)
> )
It would be nice if you added a short comment here, explaining the
fact that we're generating the file on the fly starting from a
template.
> - initrd_inject = os.path.join(base, "configs", install_config)
> + initrd_template = os.path.join(base, "configs", install_config)
> + with open(initrd_template) as template:
Missing "r" here. It's probably the default, but we're passing it
explicitly to open() everywhere else so let's be consistent.
> + content = template.read()
> +
> + tempdir = tempfile.mkdtemp()
> + initrd_inject = os.path.join(tempdir, install_config)
> +
> + open(initrd_inject, "w").write(content)
Shouldn't this also use a 'with' clause to ensure the file is closed
correctly after writing to it?
> @@ -587,6 +596,8 @@ class Application:
> except Exception as ex:
> raise Exception("Failed to install '{}': {}".format(host, ex))
>
> + shutil.rmtree(tempdir, ignore_errors=True)
So if we error out at any point, the temporary directory will be left
behind... Eh, I can live with that :)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
