Re: [Xen-devel] [PATCH v12 29/33] ts-freebsd-host-install: add arguments to test memdisk append options

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[PATCH v12 29/33] ts-freebsd-host-install: add 
arguments to test memdisk append options"):
> This is needed in order to figure out which memdisk options should be
> used to boot the images on each specific box.
> 
> Note that when passed the --recordappend argument upon success the
> script stores the tentative host property in the runvars.

Last time, I said:

  Roger Pau Monne writes ("[PATCH OSSTEST v2 07/11] ts-freebsd-host-install: 
add arguments to test memdisk append options"):
  > This is needed in order to figure out which memdisk options should be
  > used to boot the images on each specific box.
  > 
  > Note that upon success the script stores the tentative host property
  > in the runvars.
  ...
  > +    } elsif ($ARGV[0] eq "--recordappend") {
  > +        $record_append = 1;
  ...
  > +if ($bootonly) {
  > +    hostprop_putative_record($ho, "MemdiskAppend", $memdisk_append)
  > +        if $record_append;
  > +    exit 0;

  This is surely wrong.

The same code seems to be here, unchanged:

> +if ($bootonly) {
> +    hostprop_putative_record($ho, "MemdiskAppend", $memdisk_append)
> +        if $record_append;
> +    exit 0;
> +}

What I mean is that you only do the work for $record_append if
$bootonly is also set.  If --record-append is meaningful only with
--test-boot then you should die if it's specified without --test-boot.

Also, I have just noticed that the option names ought to be
   --record-append --test-boot --memdisk-append
(ie with the hyphens that are conventional in multi-word long option
names).

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel