On Mon, Mar 25, 2013 at 02:34:01PM +0000, Peter Maydell wrote:
> On 25 March 2013 14:10, Peter Lieven <p...@kamp.de> wrote:
> > Am 25.03.2013 um 15:00 schrieb Stefan Hajnoczi <stefa...@gmail.com>:
> >> On Thu, Mar 21, 2013 at 01:07:11PM +0100, Peter Lieven wrote:
> >>> this patch adds a check for qemu_find_opts("iscsi") returning
> >>> NULL instead of blindly passing the result to qemu_opts_parse().
> >>>
> >>> Signed-off-by: Peter Lieven <p...@kamp.de>
> >>> ---
> >>> vl.c | 9 ++++++---
> >>> 1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> Why is this change necessary?
> >
> > a) just to make sure we check the result of qemu_find_opts for NULL in case
> > a bug stops registering the opts again
>
> It's OK to crash for "can't happen" cases IMHO.
>
> > b) to have an error output if libiscsi is not compiled in and someone passes
> > an iscsi flag (analogue to the error if you supply -spice XXX)
>
> I don't have a strong opinion here but it would be good to be
> consistent. At the moment (as well as iscsi) SLIRP, TPM and
> mem_prealloc options all just vanish if qemu wasn't configured
> with them supported. [Various other things like SDL and Windows
> specific options do remain to produce an error.] So maybe we
> should move all these options to "always exist but may produce
> an error".
Agreed. Either this patch should be dropped because it's not strictly
necessary. Or we should be consistent and clean up all of vl.c:main().
There's not much point of just changing -iscsi.
Stefan
