Libguestfs - Mar 2016 - Re: [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

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

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.
>