Libguestfs - Jul 2016 - Re: [PATCH RFC supermin] ext2_initrd: error out if we can't add anything

From: Chen Hanxiao <chenhanxiao@gmail.com>

  If we failed to add something to initrd, just error out.

Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
---
 src/ext2_initrd.ml | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/ext2_initrd.ml b/src/ext2_initrd.ml
index d4a4e2f..d9a3a99 100644
--- a/src/ext2_initrd.ml
+++ b/src/ext2_initrd.ml
@@ -151,8 +151,12 @@ let rec build_initrd debug tmpdir modpath initrd    visit
topset;
   close_out chan;
 
+  let num_visted = StringSet.cardinal !visited in
   if debug >= 1 then
-    printf "supermin: ext2: wrote %d modules to minimal initrd\n%!"
(StringSet.cardinal !visited);
+    printf "supermin: ext2: wrote %d modules to minimal initrd\n%!"
num_visted;
+
+  if num_visited == 0 then
+    error "failed to write any modules into minimal initrd, try
'depmod -a'"
 
   (* This is the binary blob containing the init "script". *)
   let init = binary_init () in
-- 
1.8.3.1

On Fri, Jul 22, 2016 at 03:59:58PM +0800, Chen Hanxiao
wrote:
> From: Chen Hanxiao <chenhanxiao@gmail.com>
> 
>   If we failed to add something to initrd, just error out.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
> ---
>  src/ext2_initrd.ml | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ext2_initrd.ml b/src/ext2_initrd.ml
> index d4a4e2f..d9a3a99 100644
> --- a/src/ext2_initrd.ml
> +++ b/src/ext2_initrd.ml
> @@ -151,8 +151,12 @@ let rec build_initrd debug tmpdir modpath initrd > 
visit topset;
>    close_out chan;
>  
> +  let num_visted = StringSet.cardinal !visited in
>    if debug >= 1 then
> -    printf "supermin: ext2: wrote %d modules to minimal
initrd\n%!" (StringSet.cardinal !visited);
> +    printf "supermin: ext2: wrote %d modules to minimal
initrd\n%!" num_visted;
> +
> +  if num_visited == 0 then
> +    error "failed to write any modules into minimal initrd, try
'depmod -a'"
NACK.  It's not a bug if no modules are added, since all features
needed might be compiled into the kernel.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On Friday, 22 July 2016 15:59:58 CEST Chen Hanxiao
wrote:
> From: Chen Hanxiao <chenhanxiao@gmail.com>
> 
>   If we failed to add something to initrd, just error out.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
> ---
>  src/ext2_initrd.ml | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ext2_initrd.ml b/src/ext2_initrd.ml
> index d4a4e2f..d9a3a99 100644
> --- a/src/ext2_initrd.ml
> +++ b/src/ext2_initrd.ml
> @@ -151,8 +151,12 @@ let rec build_initrd debug tmpdir modpath initrd > 
visit topset;
>    close_out chan;
>  
> +  let num_visted = StringSet.cardinal !visited in
>    if debug >= 1 then
> -    printf "supermin: ext2: wrote %d modules to minimal
initrd\n%!" (StringSet.cardinal !visited);
> +    printf "supermin: ext2: wrote %d modules to minimal
initrd\n%!" num_visted;
> +
> +  if num_visited == 0 then
> +    error "failed to write any modules into minimal initrd, try
'depmod -a'"
>  
>    (* This is the binary blob containing the init "script". *)
>    let init = binary_init () in
If the "visited" set is empty, most probably also "topset"
was empty
too: for each module in "topset" there is at least one element added
in
"visited" (the module itself).

Keeping into account also what Rich replied as well, I guess a warning
could be added earlier.

Thanks,
-- 
Pino Toscano

At 2016-07-22 16:41:43, "Richard W.M. Jones" <rjones@redhat.com>
wrote:
>On Fri, Jul 22, 2016 at 03:59:58PM +0800, Chen Hanxiao wrote:
>> From: Chen Hanxiao <chenhanxiao@gmail.com>
>> 
>>   If we failed to add something to initrd, just error out.
>> 
>> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
>> ---
>>  src/ext2_initrd.ml | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/ext2_initrd.ml b/src/ext2_initrd.ml
>> index d4a4e2f..d9a3a99 100644
>> --- a/src/ext2_initrd.ml
>> +++ b/src/ext2_initrd.ml
>> @@ -151,8 +151,12 @@ let rec build_initrd debug tmpdir modpath initrd
>>    visit topset;
>>    close_out chan;
>>  
>> +  let num_visted = StringSet.cardinal !visited in
>>    if debug >= 1 then
>> -    printf "supermin: ext2: wrote %d modules to minimal
initrd\n%!" (StringSet.cardinal !visited);
>> +    printf "supermin: ext2: wrote %d modules to minimal
initrd\n%!" num_visted;
>> +
>> +  if num_visited == 0 then
>> +    error "failed to write any modules into minimal initrd, try
'depmod -a'"
>
>NACK.  It's not a bug if no modules are added, since all features
>needed might be compiled into the kernel.
>
Hi, Rich, Pino:

  Thanks for the review.
  How about a warning instead of error out?

 +  if num_visited == 0 then
 +    printf "Warnig: No modules wrote into minimal initrd, try 'depmod
-a' if needed."

Regards,

- Chen