From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MF2Xh-0000Yq-3k
	for qemu-devel@nongnu.org; Fri, 12 Jun 2009 04:55:21 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MF2Xc-0000SY-By
	for qemu-devel@nongnu.org; Fri, 12 Jun 2009 04:55:20 -0400
Received: from [199.232.76.173] (port=48272 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MF2Xc-0000SV-5S
	for qemu-devel@nongnu.org; Fri, 12 Jun 2009 04:55:16 -0400
Received: from mx2.redhat.com ([66.187.237.31]:60559)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <kraxel@redhat.com>) id 1MF2Xb-0005Mb-Ll
	for qemu-devel@nongnu.org; Fri, 12 Jun 2009 04:55:15 -0400
Message-ID: <4A321769.5090904@redhat.com>
Date: Fri, 12 Jun 2009 10:52:57 +0200
From: Gerd Hoffmann <kraxel@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] Monitor command pci_add regressed (qdev)
References: <87bpowhtmm.fsf@pike.pond.sub.org>
In-Reply-To: <87bpowhtmm.fsf@pike.pond.sub.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, paul@codesourcery.com

On 06/10/09 19:43, Markus Armbruster wrote:
> gdb --args qemu -monitor stdio tmp.qcow2 -S

> (qemu) pci_add pci_addr=auto storage if=virtio,file=foo.img
> Program received signal SIGSEGV, Segmentation fault.

> Culprit seems to be commit 07e3af9a.  qdev_init_bdrv() fails, and
> virtio_blk_init() doesn't check the failure.
>
> I haven't investigated why qdev_init_bdrv() fails (the old code got the
> BlockDriverState just fine).  Regardless, there are scenarios where
> qdev_init_bdrv() rightly fails, so virtio_blk_init() needs fixing.
>
> Returning NULL would be easy enough, but its caller
> virtio_blk_init_pci() doesn't check its value, and it is a qdev init()
> callback, which can't fail.  How to handle the error?  exit(1) would be
> just fine for -drive, but not for a monitor command.

I think the correct long-term fix is to cleanly separate the guest-side 
stuff (virtio or whatever else block device) and the host-side 
(BlockDriverState) and how the two are linked together.

The BlockDriverState setup can fail for a number of reasons.
The virtio blk setup shouldn't fail.  So setting up BlockDriverState 
first, when succeeded setup virtio-blk, then link the two should work fine.

Dunno how much work that would be with the current qemu code structure 
though.  Maybe we should allow device initialization fail for the time 
being.

cheers,
   Gerd