8:04 a.m.
On Wednesday, 12 April 2017 14:33:12 CEST Cédric Bosdonnat wrote:
virt-builder-repository allows users to easily create or update
a virt-builder source repository out of disk images. The tool can
be run in either interactive or automated mode.
---
Many good progresses, thanks :) So far worked fine in my few attempts
when playing with it -- some notes below.
+let increment_revision revision =
+ match revision with
+ | Utils.Rev_int n -> Utils.Rev_int (n + 1)
+ | Utils.Rev_string s ->
+ if Str.string_match (Str.regexp "^\\(.*[-._]\\)\\([0-9]+\\)$") s 0 then
+ let prefix = Str.matched_group 1 s in
+ let suffix = int_of_string (Str.matched_group 2 s) in
+ Utils.Rev_string (prefix ^ (string_of_int (suffix + 1)))
+ else
+ Utils.Rev_string (s ^ ".1")
The "top-level match" of this function can be simplified:
let increment_revision = function
| Utils.Rev_int n -> Utils.Rev_int (n + 1)
| Utils.Rev_string s ->
etc...
+let checksums_get_sha512 checksums =
+ match checksums with
+ | Some csums -> (
+ try
+ List.find (
+ fun c ->
+ match c with
+ | Checksums.SHA512 _ -> true
+ | _ -> false
+ ) csums
+ with
+ | _ -> Checksums.SHA512 ""
+ )
+ | None -> Checksums.SHA512 ""
Ditto, in two places:
let checksums_get_sha512 = function
| Some csums ->
try
List.find (
function
| Checksums.SHA512 _ -> true
| _ -> false
) csums
with
Not_found -> Checksums.SHA512 ""
| None -> Checksums.SHA512 ""
Also, List.find is documented to raise only Not_found, so catch that
exception specifically (to avoid a catch-all statements that could
swallow other exceptions, in case the code is expanded in the future).
+let compress_to file outdir =
+ let outimg = outdir // (Filename.basename file) ^ ".xz" in
+
+ info "Compressing ...%!";
+ let cmd = sprintf "cat \"%s\" | xz -f --best --block-size=16777216 -
>\"%s\""
+ file outimg in
+ if shell_command cmd <> 0 then
This will be done in a much better way once
https://www.redhat.com/archives/libguestfs/2017-April/msg00097.html
is in.
+let compute_short_id distro major minor =
+ match distro with
+ | "sles"
+ | "sled" -> sprintf "%s%dsp%d" distro major minor
+ | "fedora" -> sprintf "%s%d" distro major
+ | _ -> sprintf "%s%d.%d" distro major minor
I guess it will product a wrong id for e.g. sles 12.0 ;) Also fedora
also uses the major version only. A fixed version of the above, that
handles more distros, could be:
let compute_short_id distro major minor =
match distro with
| "centos" when major >= 7 ->
sprintf "%s%d.0" distro major
| "debian" when major >= 4 ->
sprintf "%s%d" distro major
| ("fedora"|"mageia") ->
sprintf "%s%d" distro major
| "sles" when major = 0 ->
sprintf "%s%d" distro major
| "sles" ->
sprintf "%s%dsp%d" distro major minor
| "ubuntu" ->
sprintf "%s%d.%02d" distro major minor
| _ (* Any other combination. *) ->
sprintf "%s%d.%d" distro major minor
(Also, note that libguestfs considers both SLES and SLED as "sles" --
maybe we could change that, but surely not as part of this series :) )
+let process_image acc_entries filename repo tmprepo index
interactive
+ no_compression sigchecker =
+ message (f_"Preparing %s") filename;
+
+ let filepath = repo // filename in
+ let { format = format; size = size } = get_disk_image_info filepath in
+ let out_path = if no_compression then
+ filepath
+ else
+ compress_to filepath tmprepo in
A bit confusing indentation for this block -- better something like:
let out_path =
if no_compression then filepath
else compress_to filepath tmprepo in
+ let out_filename = Filename.basename out_path in
+ let checksum = Checksums.compute_checksum "sha512" out_path in
+ let compressed_size = (Unix.LargeFile.stat out_path).Unix.LargeFile.st_size in
+
+ let ask ?default ?values message =
+ let default_str = match default with
+ | None -> ""
+ | Some x -> sprintf " [%s] " x in
+
+ let list_str = match values with
+ | None -> ""
+ | Some x ->
+ sprintf (f_"Choose one from the list below:\n %s\n")
+ (String.concat "\n " x) in
+
+ printf "%s%s%s" message default_str list_str;
+
+ let value = read_line () in
+ match value with
+ | "" -> default
+ | s -> Some s
I'd map also "-" -> None, otherwise it is impossible to choose to not
set
a value for a field when a default is provided. Note this needs custom
handling in ask_id, since an ID is required in interactive mode.
+ let ask_osinfo default =
+ match ask (s_ "osinfo short ID (can be found with osinfo-query os command):
")
+ ~default with
I'd maybe drop the part in parenthesis, and leave the longer
description/hints in the documentation.
+ let arch =
+ if arch = "" then (
+ if interactive then ask_arch inspected_arch
+ else inspected_arch;
+ ) else arch in
+
+ if arch = "" then
+ error (f_"missing architecture for %s") id;
Maybe ask_arch could take care of keep asking for the architecture
until a valid is give, like ask_id does.
+ let images =
+ let is_supported_format file =
+ let extension = last_part_of file '.' in
+ match extension with
+ | Some ext -> List.mem ext [ "qcow2"; "raw";
"img" ]
+ | None -> file <> "index" in
+ let is_new file =
+ try
+ let _, { Index.checksums = checksums } =
+ List.find (
+ fun (_, { Index.file_uri = file_uri }) ->
+ Filename.basename file_uri = file
+ ) index in
+ let checksum = checksums_get_sha512 checksums in
+ let path = cmdline.repo // file in
+ let file_checksum = Checksums.compute_checksum "sha512" path in
+ checksum <> file_checksum
+ with Not_found -> true in
+ let files = Array.to_list (Sys.readdir cmdline.repo) in
Please filter 'files' here to ignore anything which is not a regular
file (eg directories).
+ (* GPG sign the generated index *)
+ (match cmdline.gpgkey with
+ | None ->
+ debug "Skip index signing"
+ | Some gpgkey ->
+ message (f_"Signing index with GPG key %s") gpgkey;
+ let cmd = sprintf "%s --armor --output %s/index.gpg --export %s"
+ cmdline.gpg tmprepo gpgkey in
Please quote the paths here -- you will need to quote then as whole:
(quote (cmdline.gpg // "index.gpg"))
+ if shell_command cmd <> 0 then
+ error (f_"failed to export GPG key %s") gpgkey;
+
+ let cmd = sprintf "%s --armor --default-key %s --clearsign %s/index"
+ cmdline.gpg gpgkey tmprepo in
Ditto.
+=head1 DESCRIPTION
+
+Virt-builder is a tool for quickly building new virtual machines. It can
+be configured to use template repositories. However creating and
+maintaining a repository involves many tasks which can be automated.
+virt-builder-repository is a tool helping to manage these repositories.
+
+Virt-builder-repository loops over the files in the C</path/to/repository>
I'd just say "... in the directory specified as argument, ..."
+folder, compresses the files with a name ending by C<qcow2>,
C<raw> or
+C<img>
Mention also those without extensions are read.
+As in the repository creation use case, a minimal fragment can be
+added to the index file for the automated mode. This can be done
+on the signed index even if it may sound a strange idea: the index
+will be resigned by the tool.
"resign" means something else, I'd use "signed again".
+=item B<-K> KEYID
+
+=item B<--gpg-key> KEYID
+
+Specify the GPG key to be used to sign the repository index file.
+If not provided, the index will left unsigned. C<KEYID> is used to
+identify the GPG key to use. This value is passed to gpg's
+C<--default-key> option and can can thus be an email address or a
+fingerprint.
Double "can".
+B<NOTE>: by default, virt-builder-repository searches for the
key
+in the user's GPG key ring.
"keyring".
Thanks,
--
Pino Toscano