On Mon, Jun 6, 2011 at 11:00 AM, Kevin Wolf <kw...@redhat.com> wrote: > Am 02.06.2011 00:34, schrieb Stefan Hajnoczi: >> On Wed, Jun 1, 2011 at 1:05 PM, Kevin Wolf <kw...@redhat.com> wrote: >>> Block drivers that don't support creating images don't have a size option. >>> Fail >>> gracefully instead of segfaulting when trying to access the option's value. >>> >>> Signed-off-by: Kevin Wolf <kw...@redhat.com> >>> --- >>> block.c | 5 +++-- >>> 1 files changed, 3 insertions(+), 2 deletions(-) >> >> A command-line to reproduce the crash would be nice. > > qemu-img create -f bochs nbd:foo 32M > > It doesn't happen with a file protocol any more since we merge the > create options of the protocol with those of the format (was introduced > with Sheepdog).
Interesting, I was just looking at the format + protocol create_options merging because I tried to replace QEMUOptionParameter with QemuOpts. The idea was to make the block layer use QemuOpts and then introduce a BlockDriver open_options QemuOptsList so that image format parameters like image streaming or copy-on-read can be specified on launch. Here is where I got to: http://repo.or.cz/w/qemu/stefanha.git/commitdiff/b49babb2c8b476a36357cfd7276ca45a11039ca5 The main thing stopping me from dropping QEMUOptionParameter completely is this merging behavior. Any suggestions? >> I noticed this line above your fix: >> set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); >> >> If I follow correctly there should be an "Unknown option 'size'" error >> message before set_option_parameter_int() returns -1 (which we ignore) >> and then crash. > > Right, this is what happens. > >> Perhaps we should just catch the error when set_option_parameter_int() fails? > > We could do that, but the segfault isn't really related to a failing > set_option_parameter_int() but to the failing get_option_parameter(). I > think it's good style not to rely on the error handling of an unrelated > action some lines above. It's also bad to keep executing code after something has broken, which is the case when set_option_parameter_int() fails but we ignore the return value. Thanks for explaining, I now understand your fix better and am happy with it. Stefan
