Hello,
It looks good to me and it makes the code more robust. Removing
/usr/lib/os-release in future releases of CoreOS is less probable than
removing /usr/share/coreos/lsb-release.
After such a change I would also update
`test-data/phony-guests/make-coreos-img.sh' to also inject an os-release
file in the dummy image it creates. Find attached the
`/usr/lib/os-release' and `/usr/share/coreos/lsb-release' of the latest
stable CoreOS version to update the script accordingly.
Best Regards,
Nikos
P.S. I've checked the make-coreos.img.sh and the I've named the
temporary lsb-release file: `archlinux.release'. This does not cause any
problem but it looks weird and probably needs to be renamed.
On 30/03/16 18:16, Richard W.M. Jones wrote:
On Wed, Mar 30, 2016 at 04:38:31PM +0200, Pino Toscano wrote:
> Look for /lib/os-release in the /usr partition and try to use it, if
> present, before using lsb-release later on. This should not change the
> final result of the inspection, while using the os-release detection
> method also for CoreOS.
> ---
> Nikos, since you added the support for CoreOS in libguestfs, can you
> please check whether this change works for you as well?
> Thanks in advance.
>
>
> src/inspect-fs-unix.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
> index 35181d6..dab4370 100644
> --- a/src/inspect-fs-unix.c
> +++ b/src/inspect-fs-unix.c
> @@ -196,6 +196,8 @@ parse_os_release (guestfs_h *g, struct inspect_fs *fs, const char
*filename)
> distro = OS_DISTRO_ARCHLINUX;
> else if (VALUE_IS ("centos"))
> distro = OS_DISTRO_CENTOS;
> + else if (VALUE_IS ("coreos"))
> + distro = OS_DISTRO_COREOS;
> else if (VALUE_IS ("debian"))
> distro = OS_DISTRO_DEBIAN;
> else if (VALUE_IS ("fedora"))
> @@ -1055,6 +1057,16 @@ guestfs_int_check_coreos_usr (guestfs_h *g, struct inspect_fs
*fs)
>
> fs->type = OS_TYPE_LINUX;
> fs->distro = OS_DISTRO_COREOS;
> +
> + if (guestfs_is_file_opts (g, "/lib/os-release",
> + GUESTFS_IS_FILE_OPTS_FOLLOWSYMLINKS, 1, -1) > 0) {
> + r = parse_os_release (g, fs, "/lib/os-release");
> + if (r == -1) /* error */
> + return -1;
> + if (r == 1) /* ok - detected the release from this file */
> + goto skip_release_checks;
> + }
> +
> if (guestfs_is_file_opts (g, "/share/coreos/lsb-release",
> GUESTFS_IS_FILE_OPTS_FOLLOWSYMLINKS, 1, -1) > 0) {
> r = parse_lsb_release (g, fs, "/share/coreos/lsb-release");
> @@ -1062,6 +1074,8 @@ guestfs_int_check_coreos_usr (guestfs_h *g, struct inspect_fs
*fs)
> return -1;
> }
>
> + skip_release_checks:;
> +
> /* Determine the architecture. */
> check_architecture (g, fs);
Seems OK. Would like to hear what Nikos says.
Rich.