Introduce a new struct version with few helper functions for it: this allows to "atomically" represent a version number, without different variables to be used and checked together. Add a initialization function from a libvirt-style version number, and apply it for the qemu and libvirt versions in the direct and libvirt backends. --- src/Makefile.am | 1 + src/guestfs-internal.h | 14 +++++++++++++- src/launch-direct.c | 28 ++++++++++----------------- src/launch-libvirt.c | 43 +++++++++++++++++++++++------------------- src/version.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 99 insertions(+), 38 deletions(-) create mode 100644 src/version.c diff --git a/src/Makefile.am b/src/Makefile.am index 7a694ca..d2879f4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -133,6 +133,7 @@ libguestfs_la_SOURCES = \ umask.c \ wait.c \ whole-file.c \ + version.c \ libguestfs.syms libguestfs_la_CPPFLAGS = \ diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index d4e4e3c..098fe20 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -509,6 +509,12 @@ struct guestfs_h size_t nr_features; }; +struct version { + int v_major; + int v_minor; + int v_micro; +}; + /* Per-filesystem data stored for inspect_os. */ enum inspect_os_format { OS_FORMAT_UNKNOWN = 0, @@ -623,6 +629,12 @@ struct guestfs_message_header; struct guestfs_message_error; struct guestfs_progress; +/* version.c */ +extern void guestfs_int_version_from_libvirt (struct version *v, int vernum); +extern void guestfs_int_version_from_values (struct version *v, int maj, int min, int mic); +extern bool guestfs_int_version_is (const struct version *v, int maj, int min, int mic); +#define version_init_null(v) guestfs_int_version_from_values (v, 0, 0, 0) + /* handle.c */ extern int guestfs_int_get_backend_setting_bool (guestfs_h *g, const char *name); @@ -903,7 +915,7 @@ extern void guestfs_int_cleanup_cmd_close (struct command **); /* launch-direct.c */ extern char *guestfs_int_drive_source_qemu_param (guestfs_h *g, const struct drive_source *src); -extern bool guestfs_int_discard_possible (guestfs_h *g, struct drive *drv, unsigned long qemu_version); +extern bool guestfs_int_discard_possible (guestfs_h *g, struct drive *drv, const struct version *qemu_version); extern char *guestfs_int_qemu_escape_param (guestfs_h *g, const char *param); /* launch-*.c constructors */ diff --git a/src/launch-direct.c b/src/launch-direct.c index 1b012cf..832b975 100644 --- a/src/launch-direct.c +++ b/src/launch-direct.c @@ -62,7 +62,7 @@ struct backend_direct_data { char *qemu_devices; /* Output of qemu -device ? */ /* qemu version (0, 0 if unable to parse). */ - int qemu_version_major, qemu_version_minor; + struct version qemu_version; int virtio_scsi; /* See function qemu_supports_virtio_scsi */ @@ -420,8 +420,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) if (qemu_supports (g, data, "-no-hpet")) { ADD_CMDLINE ("-no-hpet"); } - if (data->qemu_version_major < 1 || - (data->qemu_version_major == 1 && data->qemu_version_minor <= 2)) + if (!guestfs_int_version_is (&data->qemu_version, 1, 3, 0)) ADD_CMDLINE ("-no-kvm-pit-reinjection"); else { /* New non-deprecated way, added in qemu >= 1.3. */ @@ -472,8 +471,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) if (!drv->overlay) { const char *discard_mode = ""; - int major = data->qemu_version_major, minor = data->qemu_version_minor; - unsigned long qemu_version = major * 1000000 + minor * 1000; switch (drv->discard) { case discard_disable: @@ -483,14 +480,14 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) */ break; case discard_enable: - if (!guestfs_int_discard_possible (g, drv, qemu_version)) + if (!guestfs_int_discard_possible (g, drv, &data->qemu_version)) goto cleanup0; /*FALLTHROUGH*/ case discard_besteffort: /* I believe from reading the code that this is always safe as * long as qemu >= 1.5. */ - if (major > 1 || (major == 1 && minor >= 5)) + if (guestfs_int_version_is (&data->qemu_version, 1, 5, 0)) discard_mode = ",discard=unmap"; break; } @@ -1002,8 +999,7 @@ parse_qemu_version (guestfs_h *g, struct backend_direct_data *data) CLEANUP_FREE char *major_s = NULL, *minor_s = NULL; int major_i, minor_i; - data->qemu_version_major = 0; - data->qemu_version_minor = 0; + version_init_null (&data->qemu_version); if (!data->qemu_help) return; @@ -1023,8 +1019,7 @@ parse_qemu_version (guestfs_h *g, struct backend_direct_data *data) if (minor_i == -1) goto parse_failed; - data->qemu_version_major = major_i; - data->qemu_version_minor = minor_i; + guestfs_int_version_from_values (&data->qemu_version, major_i, minor_i, 0); debug (g, "qemu version %d.%d", major_i, minor_i); } @@ -1094,7 +1089,7 @@ static int old_or_broken_virtio_scsi (guestfs_h *g, struct backend_direct_data *data) { /* qemu 1.1 claims to support virtio-scsi but in reality it's broken. */ - if (data->qemu_version_major == 1 && data->qemu_version_minor < 2) + if (!guestfs_int_version_is (&data->qemu_version, 1, 2, 0)) return 1; return 0; @@ -1384,22 +1379,19 @@ guestfs_int_drive_source_qemu_param (guestfs_h *g, const struct drive_source *sr * It returns 0 if not possible and sets the error to the reason why. * * This function is called when the user set discard == "enable". - * - * qemu_version is the version of qemu in the form returned by libvirt: - * major * 1,000,000 + minor * 1,000 + release */ bool guestfs_int_discard_possible (guestfs_h *g, struct drive *drv, - unsigned long qemu_version) + const struct version *qemu_version) { /* qemu >= 1.5. This was the first version that supported the * discard option on -drive at all. */ - bool qemu15 = qemu_version >= 1005000; + bool qemu15 = guestfs_int_version_is (qemu_version, 1, 5, 0); /* qemu >= 1.6. This was the first version that supported unmap on * qcow2 backing files. */ - bool qemu16 = qemu_version >= 1006000; + bool qemu16 = guestfs_int_version_is (qemu_version, 1, 6, 0); if (!qemu15) NOT_SUPPORTED (g, false, diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index dba28b4..6d6e162 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -118,8 +118,8 @@ struct backend_libvirt_data { char *network_bridge; char name[DOMAIN_NAME_LEN]; /* random name */ bool is_kvm; /* false = qemu, true = kvm (from capabilities)*/ - unsigned long libvirt_version; /* libvirt version */ - unsigned long qemu_version; /* qemu version (from libvirt) */ + struct version libvirt_version; /* libvirt version */ + struct version qemu_version; /* qemu version (from libvirt) */ struct secret *secrets; /* list of secrets */ size_t nr_secrets; char *uefi_code; /* UEFI (firmware) code and variables. */ @@ -253,6 +253,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) int r; uint32_t size; CLEANUP_FREE void *buf = NULL; + unsigned long version_number; params.current_proc_is_root = geteuid () == 0; @@ -262,13 +263,16 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) return -1; } - virGetVersion (&data->libvirt_version, NULL, NULL); - debug (g, "libvirt version = %lu (%lu.%lu.%lu)", - data->libvirt_version, - data->libvirt_version / 1000000UL, - data->libvirt_version / 1000UL % 1000UL, - data->libvirt_version % 1000UL); - if (data->libvirt_version < MIN_LIBVIRT_VERSION) { + virGetVersion (&version_number, NULL, NULL); + guestfs_int_version_from_libvirt (&data->libvirt_version, version_number); + debug (g, "libvirt version = %lu (%d.%d.%d)", + version_number, + data->libvirt_version.v_major, + data->libvirt_version.v_minor, + data->libvirt_version.v_micro); + if (!guestfs_int_version_is (&data->libvirt_version, + MIN_LIBVIRT_MAJOR, MIN_LIBVIRT_MINOR, + MIN_LIBVIRT_MICRO)) { error (g, _("you must have libvirt >= %d.%d.%d " "to use the 'libvirt' backend"), MIN_LIBVIRT_MAJOR, MIN_LIBVIRT_MINOR, MIN_LIBVIRT_MICRO); @@ -315,16 +319,17 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) virConnSetErrorFunc (conn, NULL, ignore_errors); /* Get hypervisor (hopefully qemu) version. */ - if (virConnectGetVersion (conn, &data->qemu_version) == 0) { - debug (g, "qemu version (reported by libvirt) = %lu (%lu.%lu.%lu)", - data->qemu_version, - data->qemu_version / 1000000UL, - data->qemu_version / 1000UL % 1000UL, - data->qemu_version % 1000UL); + if (virConnectGetVersion (conn, &version_number) == 0) { + guestfs_int_version_from_libvirt (&data->qemu_version, version_number); + debug (g, "qemu version (reported by libvirt) = %lu (%d.%d.%d)", + version_number, + data->qemu_version.v_major, + data->qemu_version.v_minor, + data->qemu_version.v_micro); } else { libvirt_debug (g, "unable to read qemu version from libvirt"); - data->qemu_version = 0; + version_init_null (&data->qemu_version); } debug (g, "get libvirt capabilities"); @@ -1259,7 +1264,7 @@ construct_libvirt_xml_devices (guestfs_h *g, * requires Cole Robinson's patch to permit /dev/urandom to be * used, which was added in libvirt 1.3.4. */ - if (params->data->libvirt_version >= 1003004) { + if (guestfs_int_version_is (&params->data->libvirt_version, 1, 3, 4)) { start_element ("rng") { attribute ("model", "virtio"); start_element ("backend") { @@ -1574,14 +1579,14 @@ construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, */ break; case discard_enable: - if (!guestfs_int_discard_possible (g, drv, data->qemu_version)) + if (!guestfs_int_discard_possible (g, drv, &data->qemu_version)) return -1; /*FALLTHROUGH*/ case discard_besteffort: /* I believe from reading the code that this is always safe as * long as qemu >= 1.5. */ - if (data->qemu_version >= 1005000) + if (guestfs_int_version_is (&data->qemu_version, 1, 5, 0)) discard_unmap = true; break; } diff --git a/src/version.c b/src/version.c new file mode 100644 index 0000000..2d0fe30 --- /dev/null +++ b/src/version.c @@ -0,0 +1,51 @@ +/* libguestfs + * Copyright (C) 2016 Red Hat Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <config.h> + +#include "guestfs.h" +#include "guestfs-internal.h" + +/** + * This file provides simple version number management. + */ + +#define VERSION_TO_NUMBER(maj, min, mic) ((maj) * 1000000 + (min) * 1000 + (mic)) +#define VERSION_STRUCT_TO_NUMBER(v) VERSION_TO_NUMBER((v)->v_major, (v)->v_minor, (v)->v_micro) + +void +guestfs_int_version_from_libvirt (struct version *v, int vernum) +{ + v->v_major = vernum / 1000000UL; + v->v_minor = vernum / 1000UL % 1000UL; + v->v_micro = vernum % 1000UL; +} + +void +guestfs_int_version_from_values (struct version *v, int maj, int min, int mic) +{ + v->v_major = maj; + v->v_minor = min; + v->v_micro = mic; +} + +bool +guestfs_int_version_is (const struct version *v, int maj, int min, int mic) +{ + return VERSION_STRUCT_TO_NUMBER(v) >= VERSION_TO_NUMBER(maj, min, mic); +} -- 2.5.5

Use the version struct in inspect_fs to hold the version of a filesystem, adapting the inspection code to that. Also, move the parts of the version parsing to helper functions of the version struct, so common bits like parsing "X.Y" or "X" version strings is done only once. --- src/guestfs-internal.h | 8 +- src/inspect-fs-cd.c | 32 +++---- src/inspect-fs-unix.c | 220 +++++++++++++++++------------------------------ src/inspect-fs-windows.c | 20 ++--- src/inspect-fs.c | 33 ++----- src/inspect-icon.c | 8 +- src/inspect.c | 9 +- src/version.c | 94 ++++++++++++++++++++ 8 files changed, 214 insertions(+), 210 deletions(-) diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 098fe20..9681def 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -605,8 +605,7 @@ struct inspect_fs { enum inspect_os_package_management package_management; char *product_name; char *product_variant; - int major_version; - int minor_version; + struct version version; char *arch; char *hostname; char *windows_systemroot; @@ -632,8 +631,13 @@ struct guestfs_progress; /* version.c */ extern void guestfs_int_version_from_libvirt (struct version *v, int vernum); extern void guestfs_int_version_from_values (struct version *v, int maj, int min, int mic); +extern int guestfs_int_version_from_x_y (guestfs_h *g, struct version *v, const char *str); +extern int guestfs_int_version_from_x_y_re (guestfs_h *g, struct version *v, const char *str, const pcre *re); +extern int guestfs_int_version_from_x_y_or_x (guestfs_h *g, struct version *v, const char *str); extern bool guestfs_int_version_is (const struct version *v, int maj, int min, int mic); +extern bool guestfs_int_version_cmp_ge (const struct version *a, const struct version *b); #define version_init_null(v) guestfs_int_version_from_values (v, 0, 0, 0) +#define version_is_null(v) ((v)->v_major == 0 && (v)->v_minor == 0 && (v)->v_micro == 0) /* handle.c */ extern int guestfs_int_get_backend_setting_bool (guestfs_h *g, const char *name); diff --git a/src/inspect-fs-cd.c b/src/inspect-fs-cd.c index b008f58..d8373f6 100644 --- a/src/inspect-fs-cd.c +++ b/src/inspect-fs-cd.c @@ -217,9 +217,9 @@ check_fedora_installer_root (guestfs_h *g, struct inspect_fs *fs) return -1; if (r > 0) { v = find_value (str); - fs->major_version = guestfs_int_parse_unsigned_int_ignore_trailing (g, v); + fs->version.v_major = guestfs_int_parse_unsigned_int_ignore_trailing (g, v); free (str); - if (fs->major_version == -1) + if (fs->version.v_major == -1) return -1; } @@ -286,10 +286,10 @@ check_isolinux_installer_root (guestfs_h *g, struct inspect_fs *fs) return -1; if (r > 0) { fs->distro = OS_DISTRO_FEDORA; - fs->major_version = + fs->version.v_major = guestfs_int_parse_unsigned_int_ignore_trailing (g, &str[29]); free (str); - if (fs->major_version == -1) + if (fs->version.v_major == -1) return -1; } @@ -301,10 +301,10 @@ check_isolinux_installer_root (guestfs_h *g, struct inspect_fs *fs) return -1; if (r > 0) { fs->distro = OS_DISTRO_RHEL; - fs->major_version = + fs->version.v_major = guestfs_int_parse_unsigned_int_ignore_trailing (g, &str[47]); free (str); - if (fs->major_version == -1) + if (fs->version.v_major == -1) return -1; } @@ -316,10 +316,10 @@ check_isolinux_installer_root (guestfs_h *g, struct inspect_fs *fs) return -1; if (r > 0) { fs->distro = OS_DISTRO_RHEL; - fs->major_version = + fs->version.v_major = guestfs_int_parse_unsigned_int_ignore_trailing (g, &str[26]); free (str); - if (fs->major_version == -1) + if (fs->version.v_major == -1) return -1; } @@ -331,10 +331,10 @@ check_isolinux_installer_root (guestfs_h *g, struct inspect_fs *fs) return -1; if (r > 0) { fs->distro = OS_DISTRO_ORACLE_LINUX; - fs->major_version = + fs->version.v_major = guestfs_int_parse_unsigned_int_ignore_trailing (g, &str[42]); free (str); - if (fs->major_version == -1) + if (fs->version.v_major == -1) return -1; } @@ -393,9 +393,9 @@ check_w2k3_installer_root (guestfs_h *g, struct inspect_fs *fs, if (r > 0) { trim_cr (str); v = find_value (str); - fs->major_version = guestfs_int_parse_unsigned_int_ignore_trailing (g, v); + fs->version.v_major = guestfs_int_parse_unsigned_int_ignore_trailing (g, v); free (str); - if (fs->major_version == -1) + if (fs->version.v_major == -1) return -1; } @@ -407,9 +407,9 @@ check_w2k3_installer_root (guestfs_h *g, struct inspect_fs *fs, if (r > 0) { trim_cr (str); v = find_value (str); - fs->minor_version = guestfs_int_parse_unsigned_int_ignore_trailing (g, v); + fs->version.v_minor = guestfs_int_parse_unsigned_int_ignore_trailing (g, v); free (str); - if (fs->minor_version == -1) + if (fs->version.v_minor == -1) return -1; } @@ -531,8 +531,8 @@ guestfs_int_check_installer_iso (guestfs_h *g, struct inspect_fs *fs, fs->distro = osinfo->distro; fs->product_name = osinfo->product_name ? safe_strdup (g, osinfo->product_name) : NULL; - fs->major_version = osinfo->major_version; - fs->minor_version = osinfo->minor_version; + guestfs_int_version_from_values (&fs->version, osinfo->major_version, + osinfo->minor_version, 0); fs->arch = osinfo->arch ? safe_strdup (g, osinfo->arch) : NULL; fs->is_live_disk = osinfo->is_live_disk; diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c index 82d249a..8d82d56 100644 --- a/src/inspect-fs-unix.c +++ b/src/inspect-fs-unix.c @@ -54,7 +54,6 @@ COMPILE_REGEXP (re_oracle_linux_old, COMPILE_REGEXP (re_oracle_linux, "Oracle Linux.*release (\\d+)\\.(\\d+)", 0) COMPILE_REGEXP (re_oracle_linux_no_minor, "Oracle Linux.*release (\\d+)", 0) -COMPILE_REGEXP (re_major_minor, "(\\d+)\\.(\\d+)", 0) COMPILE_REGEXP (re_xdev, "^/dev/(h|s|v|xv)d([a-z]+)(\\d*)$", 0) COMPILE_REGEXP (re_cciss, "^/dev/(cciss/c\\d+d\\d+)(?:p(\\d+))?$", 0) COMPILE_REGEXP (re_mdN, "^(/dev/md\\d+)$", 0) @@ -148,7 +147,8 @@ parse_os_release (guestfs_h *g, struct inspect_fs *fs, const char *filename) size_t i; enum inspect_os_distro distro = OS_DISTRO_UNKNOWN; CLEANUP_FREE char *product_name = NULL; - int major_version = -1, minor_version = -1; + struct version version; + guestfs_int_version_from_values (&version, -1, -1, 0); /* Don't trust guestfs_read_lines not to break with very large files. * Check the file size is something reasonable first. @@ -220,43 +220,27 @@ parse_os_release (guestfs_h *g, struct inspect_fs *fs, const char *filename) free (product_name); product_name = safe_strndup (g, value, value_len); } else if (STRPREFIX (line, "VERSION_ID=")) { - char *major, *minor; - if (match2 (g, value, re_major_minor, &major, &minor)) { - major_version = guestfs_int_parse_unsigned_int (g, major); - free (major); - if (major_version == -1) { - free (minor); - return -1; - } - minor_version = guestfs_int_parse_unsigned_int (g, minor); - free (minor); - if (minor_version == -1) - return -1; - } else { - CLEANUP_FREE char *buf = - safe_asprintf (g, "%.*s", (int) value_len, value); - major_version = guestfs_int_parse_unsigned_int (g, buf); - /* Handle cases where VERSION_ID is not a number. */ - if (major_version != -1) - minor_version = 0; - } + CLEANUP_FREE char *buf = + safe_asprintf (g, "%.*s", (int) value_len, value); + if (guestfs_int_version_from_x_y_or_x (g, &version, buf) == -1) + return -1; } #undef VALUE_IS } /* If we haven't got all the fields, exit right away. */ if (distro == OS_DISTRO_UNKNOWN || product_name == NULL || - major_version == -1 || minor_version == -1) + version.v_major == -1 || version.v_minor == -1) return 0; /* Apparently, os-release in Debian and CentOS does not provide the full * version number in VERSION_ID, but just the "major" part of it. - * Hence, if minor_version is 0, act as there was no information in + * Hence, if version.v_minor is 0, act as there was no information in * os-release, which will continue the inspection using the release files * as done previously. */ if ((distro == OS_DISTRO_DEBIAN || distro == OS_DISTRO_CENTOS) && - minor_version == 0) + version.v_minor == 0) return 0; /* We got everything, so set the fields and report the inspection @@ -265,8 +249,7 @@ parse_os_release (guestfs_h *g, struct inspect_fs *fs, const char *filename) fs->distro = distro; fs->product_name = product_name; product_name = NULL; - fs->major_version = major_version; - fs->minor_version = minor_version; + fs->version = version; return 1; } @@ -351,19 +334,8 @@ parse_lsb_release (guestfs_h *g, struct inspect_fs *fs, const char *filename) r = 1; } else if (STRPREFIX (lines[i], "DISTRIB_RELEASE=")) { - char *major, *minor; - if (match2 (g, &lines[i][16], re_major_minor, &major, &minor)) { - fs->major_version = guestfs_int_parse_unsigned_int (g, major); - free (major); - if (fs->major_version == -1) { - free (minor); - return -1; - } - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); - free (minor); - if (fs->minor_version == -1) - return -1; - } + if (guestfs_int_version_from_x_y (g, &fs->version, &lines[i][16]) == -1) + return -1; } else if (fs->product_name == NULL && (STRPREFIX (lines[i], "DISTRIB_DESCRIPTION=\"") || @@ -390,7 +362,6 @@ static int parse_suse_release (guestfs_h *g, struct inspect_fs *fs, const char *filename) { int64_t size; - char *major, *minor; CLEANUP_FREE_STRING_LIST char **lines = NULL; int r = -1; @@ -419,6 +390,8 @@ parse_suse_release (guestfs_h *g, struct inspect_fs *fs, const char *filename) /* Match SLES first because openSuSE regex overlaps some SLES release strings */ if (match (g, fs->product_name, re_sles) || match (g, fs->product_name, re_nld)) { + char *major, *minor; + fs->distro = OS_DISTRO_SLES; /* Second line contains version string */ @@ -427,9 +400,9 @@ parse_suse_release (guestfs_h *g, struct inspect_fs *fs, const char *filename) major = match1 (g, lines[1], re_sles_version); if (major == NULL) goto out; - fs->major_version = guestfs_int_parse_unsigned_int (g, major); + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); free (major); - if (fs->major_version == -1) + if (fs->version.v_major == -1) goto out; /* Third line contains service pack string */ @@ -438,9 +411,9 @@ parse_suse_release (guestfs_h *g, struct inspect_fs *fs, const char *filename) minor = match1 (g, lines[2], re_sles_patchlevel); if (minor == NULL) goto out; - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); + fs->version.v_minor = guestfs_int_parse_unsigned_int (g, minor); free (minor); - if (fs->minor_version == -1) + if (fs->version.v_minor == -1) goto out; } else if (match (g, fs->product_name, re_opensuse)) { @@ -449,14 +422,9 @@ parse_suse_release (guestfs_h *g, struct inspect_fs *fs, const char *filename) /* Second line contains version string */ if (lines[1] == NULL) goto out; - if (match2 (g, lines[1], re_opensuse_version, &major, &minor)) { - fs->major_version = guestfs_int_parse_unsigned_int (g, major); - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); - free (major); - free (minor); - if (fs->major_version == -1 || fs->minor_version == -1) - goto out; - } + if (guestfs_int_version_from_x_y_re (g, &fs->version, lines[1], + re_opensuse_version) == -1) + goto out; } r = 0; @@ -509,22 +477,22 @@ guestfs_int_check_linux_root (guestfs_h *g, struct inspect_fs *fs) if (match2 (g, fs->product_name, re_oracle_linux_old, &major, &minor) || match2 (g, fs->product_name, re_oracle_linux, &major, &minor)) { - fs->major_version = guestfs_int_parse_unsigned_int (g, major); + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); free (major); - if (fs->major_version == -1) { + if (fs->version.v_major == -1) { free (minor); return -1; } - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); + fs->version.v_minor = guestfs_int_parse_unsigned_int (g, minor); free (minor); - if (fs->minor_version == -1) + if (fs->version.v_minor == -1) return -1; } else if ((major = match1 (g, fs->product_name, re_oracle_linux_no_minor)) != NULL) { - fs->major_version = guestfs_int_parse_unsigned_int (g, major); + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); free (major); - if (fs->major_version == -1) + if (fs->version.v_major == -1) return -1; - fs->minor_version = 0; + fs->version.v_minor = 0; } } else if (guestfs_is_file_opts (g, "/etc/centos-release", @@ -536,23 +504,23 @@ guestfs_int_check_linux_root (guestfs_h *g, struct inspect_fs *fs) if (match2 (g, fs->product_name, re_centos_old, &major, &minor) || match2 (g, fs->product_name, re_centos, &major, &minor)) { - fs->major_version = guestfs_int_parse_unsigned_int (g, major); + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); free (major); - if (fs->major_version == -1) { + if (fs->version.v_major == -1) { free (minor); return -1; } - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); + fs->version.v_minor = guestfs_int_parse_unsigned_int (g, minor); free (minor); - if (fs->minor_version == -1) + if (fs->version.v_minor == -1) return -1; } else if ((major = match1 (g, fs->product_name, re_centos_no_minor)) != NULL) { - fs->major_version = guestfs_int_parse_unsigned_int (g, major); + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); free (major); - if (fs->major_version == -1) + if (fs->version.v_major == -1) return -1; - fs->minor_version = 0; + fs->version.v_minor = 0; } } else if (guestfs_is_file_opts (g, "/etc/altlinux-release", @@ -562,18 +530,9 @@ guestfs_int_check_linux_root (guestfs_h *g, struct inspect_fs *fs) if (parse_release_file (g, fs, "/etc/altlinux-release") == -1) return -1; - if (match2 (g, fs->product_name, re_altlinux, &major, &minor)) { - fs->major_version = guestfs_int_parse_unsigned_int (g, major); - free (major); - if (fs->major_version == -1) { - free (minor); - return -1; - } - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); - free (minor); - if (fs->minor_version == -1) - return -1; - } + if (guestfs_int_version_from_x_y_re (g, &fs->version, fs->product_name, + re_altlinux) == -1) + return -1; } else if (guestfs_is_file_opts (g, "/etc/redhat-release", GUESTFS_IS_FILE_OPTS_FOLLOWSYMLINKS, 1, -1) > 0) { @@ -584,76 +543,76 @@ guestfs_int_check_linux_root (guestfs_h *g, struct inspect_fs *fs) if ((major = match1 (g, fs->product_name, re_fedora)) != NULL) { fs->distro = OS_DISTRO_FEDORA; - fs->major_version = guestfs_int_parse_unsigned_int (g, major); + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); free (major); - if (fs->major_version == -1) + if (fs->version.v_major == -1) return -1; } else if (match2 (g, fs->product_name, re_rhel_old, &major, &minor) || match2 (g, fs->product_name, re_rhel, &major, &minor)) { fs->distro = OS_DISTRO_RHEL; - fs->major_version = guestfs_int_parse_unsigned_int (g, major); + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); free (major); - if (fs->major_version == -1) { + if (fs->version.v_major == -1) { free (minor); return -1; } - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); + fs->version.v_minor = guestfs_int_parse_unsigned_int (g, minor); free (minor); - if (fs->minor_version == -1) + if (fs->version.v_minor == -1) return -1; } else if ((major = match1 (g, fs->product_name, re_rhel_no_minor)) != NULL) { fs->distro = OS_DISTRO_RHEL; - fs->major_version = guestfs_int_parse_unsigned_int (g, major); + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); free (major); - if (fs->major_version == -1) + if (fs->version.v_major == -1) return -1; - fs->minor_version = 0; + fs->version.v_minor = 0; } else if (match2 (g, fs->product_name, re_centos_old, &major, &minor) || match2 (g, fs->product_name, re_centos, &major, &minor)) { fs->distro = OS_DISTRO_CENTOS; - fs->major_version = guestfs_int_parse_unsigned_int (g, major); + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); free (major); - if (fs->major_version == -1) { + if (fs->version.v_major == -1) { free (minor); return -1; } - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); + fs->version.v_minor = guestfs_int_parse_unsigned_int (g, minor); free (minor); - if (fs->minor_version == -1) + if (fs->version.v_minor == -1) return -1; } else if ((major = match1 (g, fs->product_name, re_centos_no_minor)) != NULL) { fs->distro = OS_DISTRO_CENTOS; - fs->major_version = guestfs_int_parse_unsigned_int (g, major); + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); free (major); - if (fs->major_version == -1) + if (fs->version.v_major == -1) return -1; - fs->minor_version = 0; + fs->version.v_minor = 0; } else if (match2 (g, fs->product_name, re_scientific_linux_old, &major, &minor) || match2 (g, fs->product_name, re_scientific_linux, &major, &minor)) { fs->distro = OS_DISTRO_SCIENTIFIC_LINUX; - fs->major_version = guestfs_int_parse_unsigned_int (g, major); + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); free (major); - if (fs->major_version == -1) { + if (fs->version.v_major == -1) { free (minor); return -1; } - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); + fs->version.v_minor = guestfs_int_parse_unsigned_int (g, minor); free (minor); - if (fs->minor_version == -1) + if (fs->version.v_minor == -1) return -1; } else if ((major = match1 (g, fs->product_name, re_scientific_linux_no_minor)) != NULL) { fs->distro = OS_DISTRO_SCIENTIFIC_LINUX; - fs->major_version = guestfs_int_parse_unsigned_int (g, major); + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); free (major); - if (fs->major_version == -1) + if (fs->version.v_major == -1) return -1; - fs->minor_version = 0; + fs->version.v_minor = 0; } } else if (guestfs_is_file_opts (g, "/etc/debian_version", @@ -779,18 +738,9 @@ guestfs_int_check_linux_root (guestfs_h *g, struct inspect_fs *fs) if (parse_release_file (g, fs, "/etc/frugalware-release") == -1) return -1; - if (match2 (g, fs->product_name, re_frugalware, &major, &minor)) { - fs->major_version = guestfs_int_parse_unsigned_int (g, major); - free (major); - if (fs->major_version == -1) { - free (minor); - return -1; - } - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); - free (minor); - if (fs->minor_version == -1) - return -1; - } + if (guestfs_int_version_from_x_y_re (g, &fs->version, fs->product_name, + re_frugalware) == -1) + return -1; } skip_release_checks:; @@ -857,23 +807,17 @@ guestfs_int_check_netbsd_root (guestfs_h *g, struct inspect_fs *fs) if (guestfs_is_file_opts (g, "/etc/release", GUESTFS_IS_FILE_OPTS_FOLLOWSYMLINKS, 1, -1) > 0) { - char *major, *minor; + int result; if (parse_release_file (g, fs, "/etc/release") == -1) return -1; - if (match2 (g, fs->product_name, re_netbsd, &major, &minor)) { + result = guestfs_int_version_from_x_y_re (g, &fs->version, + fs->product_name, re_netbsd); + if (result == -1) { + return -1; + } else if (result == 1) { fs->type = OS_TYPE_NETBSD; fs->distro = OS_DISTRO_NETBSD; - fs->major_version = guestfs_int_parse_unsigned_int (g, major); - free (major); - if (fs->major_version == -1) { - free (minor); - return -1; - } - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); - free (minor); - if (fs->minor_version == -1) - return -1; } } else { return -1; @@ -915,12 +859,12 @@ guestfs_int_check_openbsd_root (guestfs_h *g, struct inspect_fs *fs) * OpenBSD ?.? (UNKNOWN) */ if ((fs->product_name[8] != '?') && (fs->product_name[10] != '?')) { - fs->major_version = guestfs_int_parse_unsigned_int (g, major); - if (fs->major_version == -1) + fs->version.v_major = guestfs_int_parse_unsigned_int (g, major); + if (fs->version.v_major == -1) return -1; - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); - if (fs->minor_version == -1) + fs->version.v_minor = guestfs_int_parse_unsigned_int (g, minor); + if (fs->version.v_minor == -1) return -1; } } @@ -990,22 +934,12 @@ guestfs_int_check_minix_root (guestfs_h *g, struct inspect_fs *fs) if (guestfs_is_file_opts (g, "/etc/version", GUESTFS_IS_FILE_OPTS_FOLLOWSYMLINKS, 1, -1) > 0) { - char *major, *minor; if (parse_release_file (g, fs, "/etc/version") == -1) return -1; - if (match2 (g, fs->product_name, re_minix, &major, &minor)) { - fs->major_version = guestfs_int_parse_unsigned_int (g, major); - free (major); - if (fs->major_version == -1) { - free (minor); - return -1; - } - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); - free (minor); - if (fs->minor_version == -1) - return -1; - } + if (guestfs_int_version_from_x_y_re (g, &fs->version, fs->product_name, + re_minix) == -1) + return -1; } else { return -1; } diff --git a/src/inspect-fs-windows.c b/src/inspect-fs-windows.c index 3ac9107..8399a90 100644 --- a/src/inspect-fs-windows.c +++ b/src/inspect-fs-windows.c @@ -324,7 +324,7 @@ check_windows_software_registry (guestfs_h *g, struct inspect_fs *fs) goto out; } - fs->major_version = le32toh (*(int32_t *)vbuf); + fs->version.v_major = le32toh (*(int32_t *)vbuf); /* Ignore CurrentVersion if we see it after this key. */ ignore_currentversion = true; @@ -342,7 +342,7 @@ check_windows_software_registry (guestfs_h *g, struct inspect_fs *fs) goto out; } - fs->minor_version = le32toh (*(int32_t *)vbuf); + fs->version.v_minor = le32toh (*(int32_t *)vbuf); /* Ignore CurrentVersion if we see it after this key. */ ignore_currentversion = true; @@ -351,19 +351,9 @@ check_windows_software_registry (guestfs_h *g, struct inspect_fs *fs) CLEANUP_FREE char *version = guestfs_hivex_value_utf8 (g, value); if (!version) goto out; - char *major, *minor; - if (match2 (g, version, re_windows_version, &major, &minor)) { - fs->major_version = guestfs_int_parse_unsigned_int (g, major); - free (major); - if (fs->major_version == -1) { - free (minor); - goto out; - } - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); - free (minor); - if (fs->minor_version == -1) - goto out; - } + if (guestfs_int_version_from_x_y_re (g, &fs->version, version, + re_windows_version) == -1) + goto out; } else if (STRCASEEQ (key, "InstallationType")) { fs->product_variant = guestfs_hivex_value_utf8 (g, value); diff --git a/src/inspect-fs.c b/src/inspect-fs.c index e9976cf..666cf37 100644 --- a/src/inspect-fs.c +++ b/src/inspect-fs.c @@ -36,8 +36,6 @@ #include "guestfs.h" #include "guestfs-internal.h" -COMPILE_REGEXP (re_major_minor, "(\\d+)\\.(\\d+)", 0) - static int check_filesystem (guestfs_h *g, const char *mountable, const struct guestfs_internal_mountable *m, int whole_device); @@ -439,25 +437,14 @@ guestfs_int_parse_unsigned_int_ignore_trailing (guestfs_h *g, const char *str) int guestfs_int_parse_major_minor (guestfs_h *g, struct inspect_fs *fs) { - char *major, *minor; + if (guestfs_int_version_from_x_y (g, &fs->version, fs->product_name) == -1) + return -1; - if (match2 (g, fs->product_name, re_major_minor, &major, &minor)) { - fs->major_version = guestfs_int_parse_unsigned_int (g, major); - free (major); - if (fs->major_version == -1) { - free (minor); - return -1; - } - fs->minor_version = guestfs_int_parse_unsigned_int (g, minor); - free (minor); - if (fs->minor_version == -1) - return -1; - } return 0; } /* At the moment, package format and package management is just a - * simple function of the distro and major_version fields, so these + * simple function of the distro and version.v_major fields, so these * can never return an error. We might be cleverer in future. */ void @@ -528,11 +515,11 @@ guestfs_int_check_package_management (guestfs_h *g, struct inspect_fs *fs) case OS_DISTRO_FEDORA: /* If Fedora >= 22 and dnf is installed, say "dnf". */ - if (fs->major_version >= 22 && + if (guestfs_int_version_is (&fs->version, 22, 0, 0) && guestfs_is_file_opts (g, "/usr/bin/dnf", GUESTFS_IS_FILE_OPTS_FOLLOWSYMLINKS, 1, -1) > 0) fs->package_management = OS_PACKAGE_MANAGEMENT_DNF; - else if (fs->major_version >= 1) + else if (guestfs_int_version_is (&fs->version, 1, 0, 0)) fs->package_management = OS_PACKAGE_MANAGEMENT_YUM; else /* Probably parsing the release file failed, see RHBZ#1332025. */ @@ -544,9 +531,9 @@ guestfs_int_check_package_management (guestfs_h *g, struct inspect_fs *fs) case OS_DISTRO_CENTOS: case OS_DISTRO_SCIENTIFIC_LINUX: case OS_DISTRO_ORACLE_LINUX: - if (fs->major_version >= 5) + if (guestfs_int_version_is (&fs->version, 5, 0, 0)) fs->package_management = OS_PACKAGE_MANAGEMENT_YUM; - else if (fs->major_version >= 2) + else if (guestfs_int_version_is (&fs->version, 2, 0, 0)) fs->package_management = OS_PACKAGE_MANAGEMENT_UP2DATE; else /* Probably parsing the release file failed, see RHBZ#1332025. */ @@ -732,10 +719,8 @@ guestfs_int_merge_fs_inspections (guestfs_h *g, struct inspect_fs *dst, struct i src->product_variant = NULL; } - if (dst->major_version == 0 && dst->minor_version == 0) { - dst->major_version = src->major_version; - dst->minor_version = src->minor_version; - } + if (version_is_null (&dst->version)) + dst->version = src->version; if (dst->arch == NULL) { dst->arch = src->arch; diff --git a/src/inspect-icon.c b/src/inspect-icon.c index 5c5169e..1890127 100644 --- a/src/inspect-icon.c +++ b/src/inspect-icon.c @@ -324,7 +324,7 @@ icon_rhel (guestfs_h *g, struct inspect_fs *fs, size_t *size_r) { const char *shadowman; - if (fs->major_version <= 6) + if (!guestfs_int_version_is (&fs->version, 7, 0, 0)) shadowman = "/usr/share/pixmaps/redhat/shadowman-transparent.png"; else shadowman = "/usr/share/pixmaps/fedora-logo-sprite.png"; @@ -583,15 +583,15 @@ icon_windows (guestfs_h *g, struct inspect_fs *fs, size_t *size_r) return NOT_FOUND; /* Windows XP. */ - if (fs->major_version == 5 && fs->minor_version == 1) + if (fs->version.v_major == 5 && fs->version.v_minor == 1) return icon_windows_xp (g, fs, size_r); /* Windows 7. */ - else if (fs->major_version == 6 && fs->minor_version == 1) + else if (fs->version.v_major == 6 && fs->version.v_minor == 1) return icon_windows_7 (g, fs, size_r); /* Windows 8. */ - else if (fs->major_version == 6 && fs->minor_version == 2) + else if (fs->version.v_major == 6 && fs->version.v_minor == 2) return icon_windows_8 (g, fs, size_r); /* Not (yet) a supported version of Windows. */ diff --git a/src/inspect.c b/src/inspect.c index 4d8138a..bd32d8f 100644 --- a/src/inspect.c +++ b/src/inspect.c @@ -136,10 +136,7 @@ collect_coreos_inspection_info (guestfs_h *g) * the boot loader. As a workaround, we check the OS versions and pick the * one with the higher version as active. */ - if (usr && - (usr->major_version > fs->major_version || - (usr->major_version == fs->major_version && - usr->minor_version > fs->minor_version))) + if (usr && guestfs_int_version_cmp_ge (&usr->version, &fs->version)) continue; usr = fs; @@ -310,7 +307,7 @@ guestfs_impl_inspect_get_major_version (guestfs_h *g, const char *root) if (!fs) return -1; - return fs->major_version; + return fs->version.v_major; } int @@ -320,7 +317,7 @@ guestfs_impl_inspect_get_minor_version (guestfs_h *g, const char *root) if (!fs) return -1; - return fs->minor_version; + return fs->version.v_minor; } char * diff --git a/src/version.c b/src/version.c index 2d0fe30..7015cda 100644 --- a/src/version.c +++ b/src/version.c @@ -18,6 +18,11 @@ #include <config.h> +#include <string.h> +#include <unistd.h> + +#include "ignore-value.h" + #include "guestfs.h" #include "guestfs-internal.h" @@ -25,9 +30,13 @@ * This file provides simple version number management. */ +COMPILE_REGEXP (re_major_minor, "(\\d+)\\.(\\d+)", 0) + #define VERSION_TO_NUMBER(maj, min, mic) ((maj) * 1000000 + (min) * 1000 + (mic)) #define VERSION_STRUCT_TO_NUMBER(v) VERSION_TO_NUMBER((v)->v_major, (v)->v_minor, (v)->v_micro) +static int version_from_x_y_or_x (guestfs_h *g, struct version *v, const char *str, const pcre *re, bool allow_only_x); + void guestfs_int_version_from_libvirt (struct version *v, int vernum) { @@ -44,8 +53,93 @@ guestfs_int_version_from_values (struct version *v, int maj, int min, int mic) v->v_micro = mic; } +/** + * Parses a version from a string, looking for a C<X.Y> pattern. + * + * Returns C<-1> on failure (like failed integer parsing), C<0> on missing + * match, and C<1> on match and successful parsing. C<v> is changed only + * on successful match. + */ +int +guestfs_int_version_from_x_y (guestfs_h *g, struct version *v, const char *str) +{ + return version_from_x_y_or_x (g, v, str, re_major_minor, false); +} + +/** + * Parses a version from a string, using the specified C<re> as regular + * expression which I<must> provide (at least) two matches. + * + * Returns C<-1> on failure (like failed integer parsing), C<0> on missing + * match, and C<1> on match and successful parsing. C<v> is changed only + * on successful match. + */ +int +guestfs_int_version_from_x_y_re (guestfs_h *g, struct version *v, + const char *str, const pcre *re) +{ + return version_from_x_y_or_x (g, v, str, re, false); +} + +/** + * Parses a version from a string, either looking for a C<X.Y> pattern or + * considering it as whole integer. + * + * Returns C<-1> on failure (like failed integer parsing), C<0> on missing + * match, and C<1> on match and successful parsing. C<v> is changed only + * on successful match. + */ +int +guestfs_int_version_from_x_y_or_x (guestfs_h *g, struct version *v, + const char *str) +{ + return version_from_x_y_or_x (g, v, str, re_major_minor, true); +} + bool guestfs_int_version_is (const struct version *v, int maj, int min, int mic) { return VERSION_STRUCT_TO_NUMBER(v) >= VERSION_TO_NUMBER(maj, min, mic); } + +bool +guestfs_int_version_cmp_ge (const struct version *a, const struct version *b) +{ + return VERSION_STRUCT_TO_NUMBER(a) >= VERSION_STRUCT_TO_NUMBER(b); +} + +static int +version_from_x_y_or_x (guestfs_h *g, struct version *v, const char *str, + const pcre *re, bool allow_only_x) +{ + CLEANUP_FREE char *major = NULL; + CLEANUP_FREE char *minor = NULL; + + if (match2 (g, str, re, &major, &minor)) { + int major_version, minor_version; + + major_version = guestfs_int_parse_unsigned_int (g, major); + if (major_version == -1) + return -1; + minor_version = guestfs_int_parse_unsigned_int (g, minor); + if (minor_version == -1) + return -1; + + v->v_major = major_version; + v->v_minor = minor_version; + v->v_micro = 0; + + return 1; + } else if (allow_only_x) { + int major_version = guestfs_int_parse_unsigned_int (g, str); + if (major_version == -1) + return -1; + + v->v_major = major_version; + v->v_minor = 0; + v->v_micro = 0; + + return 1; + } + return 0; +} -- 2.5.5