[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] vl.c: throw an error if iscsi is not suppor
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] vl.c: throw an error if iscsi is not supported |
Date: |
Tue, 26 Mar 2013 10:26:46 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, Mar 25, 2013 at 02:34:01PM +0000, Peter Maydell wrote:
> On 25 March 2013 14:10, Peter Lieven <address@hidden> wrote:
> > Am 25.03.2013 um 15:00 schrieb Stefan Hajnoczi <address@hidden>:
> >> 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 <address@hidden>
> >>> ---
> >>> 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
Re: [Qemu-devel] [PATCH 1/2] vl.c: call bdrv_init_with_whitelist() before cmdline parsing, Stefan Hajnoczi, 2013/03/25