Pino Toscano
2016-Mar-30 14:38 UTC
[Libguestfs] [PATCH] inspect: use os-release for CoreOS
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); -- 2.5.5
Richard W.M. Jones
2016-Mar-30 15:16 UTC
Re: [Libguestfs] [PATCH] inspect: use os-release for CoreOS
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. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Nikos Skalkotos
2016-Mar-31 09:10 UTC
Re: [Libguestfs] [PATCH] inspect: use os-release for CoreOS
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. >