Hi, Michael: On 03/28/2012 07:11 PM, Michael S. Tsirkin wrote: > On Wed, Mar 28, 2012 at 11:40:33AM +0800, Ren Mingxin wrote: >> On guest with upstream's kernel(3.3.0-rc7), I >> mounted virtblk as: >> a) # mkfs /dev/vda >> b) # mount /dev/vda /mnt >> c) # cd /mnt >> >> Then I did hotplug for virtblk via virsh on host as: >> a) # sudo virsh detach-disk guest vda >> b) # sudo virsh attach-disk guest /media/data/test.img vda > Did a quick test and I don't seem to see a panic. > How reproducible is this for you? I think the key difference of environment is udev: my udev version is 147(BTW, actually, except the upstream's kernel, other tools are based on rhel6.2ga) with "60-persistent-storage.rules": # cat /lib/udev/rules.d/60-persistent-storage.rules .... # skip unpartitioned removable media devices from drivers which do not send "change" events ENV{DEVTYPE}=="disk", KERNEL!="xvd*|sd*|sr*", ATTR{removable}=="1", GOTO="persistent_storage_end" .... # probe filesystem metadata of disks KERNEL!="xvd*|sr*", IMPORT{program}="/sbin/blkid -o udev -p $tempnode" .... LABEL="persistent_storage_end" According to this, "blkid" will be executed on unpartitioned virtblk, but will not be on partitioned virtblk. So this panic will only be occurred on unpartitioned virtblk. BTW: I commented this "blkid" line in this rule file as debug, and the panic can be avoided. >> I encountered guest's kernel panic (*probability* >> *event*)whose backtrace liked this: >> >> PID: 2496 TASK: ffff88001f5de080 CPU: 0 COMMAND: "blkid" >> #0 [ffff88001afdbb00] machine_kexec at ffffffff81031fcb >> #1 [ffff88001afdbb60] crash_kexec at ffffffff810b8f72 >> #2 [ffff88001afdbc30] oops_end at ffffffff814f04b0 >> #3 [ffff88001afdbc60] die at ffffffff8100f26b >> #4 [ffff88001afdbc90] do_general_protection at ffffffff814f0042 >> #5 [ffff88001afdbcc0] general_protection at ffffffff814ef815 >> [exception RIP: virtio_check_driver_offered_feature+27] >> RIP: ffffffffa00540cb RSP: ffff88001afdbd78 RFLAGS: 00010206 >> RAX: ffffffff810ffde0 RBX: ffff88001f14a800 RCX: 000000004cf0758b >> RDX: 4ce86d8b4ce0658b RSI: 0000000000000007 RDI: ffffffff81a970a0 >> RBP: ffff88001afdbd78 R8: ffffffffa009d060 R9: 0000000000000100 >> R10: 0000000000000006 R11: 0000000000000246 R12: 000000000000101d >> R13: 0000000000005331 R14: ffffffff81a970a0 R15: 0000000000000000 >> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 >> #6 [ffff88001afdbd80] virtblk_ioctl at ffffffffa009c459 [virtio_blk] >> #7 [ffff88001afdbdc0] __blkdev_driver_ioctl at ffffffff81257317 >> #8 [ffff88001afdbe00] blkdev_ioctl at ffffffff8125779d >> #9 [ffff88001afdbe50] block_ioctl at ffffffff811aec5c >> #10 [ffff88001afdbe60] vfs_ioctl at ffffffff81189342 >> #11 [ffff88001afdbea0] do_vfs_ioctl at ffffffff811894e4 >> #12 [ffff88001afdbf30] sys_ioctl at ffffffff81189a61 >> #13 [ffff88001afdbf80] system_call_fastpath at ffffffff8100b0f2 >> RIP: 0000003f566dd847 RSP: 00007fffa23e6130 RFLAGS: 00010202 >> RAX: 0000000000000010 RBX: ffffffff8100b0f2 RCX: 0000000000000008 >> RDX: 0000000000000000 RSI: 0000000000005331 RDI: 0000000000000003 >> RBP: 0000000040000000 R8: 0000003f5699b580 R9: 0000000000000100 >> R10: 0000000000000006 R11: 0000000000000246 R12: 0000000000000000 >> R13: 0000003f59020ba0 R14: 0000000000000003 R15: 000000000241f030 >> ORIG_RAX: 0000000000000010 CS: 0033 SS: 002b >> >> This panic was triggered by the command of "blkid" in >> udev's rule "60-persistent-storage.rules". > The weird thing is, we should not get any ioctls > before add_disk is called. The problem is, virtblk_probe() has been called to add disk, then the "blkid" in the udev rule calls virtblk_ioctl(), but at this time, the data of bdev-> bd_disk->private_data is wrong. See details in the attachment please. >> So, when the virtblk was reattached, command "blkid" >> probed filesystem metadata of disks. At that moment, >> virtblk_ioctl() was called, but the data of "virtio_blk" >> "bdev->bd_disk->private_data" pointing to may not be >> updated for the sync reason. > Sorry, what does "for the sync reason" mean? > private_data seems to be set at device probe > and never changed. Sorry, I means timing reason. For if I debug the virtblk_probe() by step, this panic will not be happened.(the data should have been updated.) >> This patch do this check to avoid panic. >> >> Any comment will be appreciated. >> >> Signed-off-by: Ren Mingxin >> --- >> virtio_blk.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c >> index c4a60ba..4ac81f8 100644 >> --- a/drivers/block/virtio_blk.c >> +++ b/drivers/block/virtio_blk.c >> @@ -245,6 +245,12 @@ static int virtblk_ioctl(struct block_device >> *bdev, fmode_t mode, >> struct virtio_blk *vblk = disk->private_data; >> >> /* >> + * Check whether the private data pointer has been updated. >> + */ >> + if (vblk != vblk->vdev->priv) >> + return -EFAULT; >> + > I would change this to > BUG_ON(vblk != vblk->vdev->priv); > > Add traces to virtblk_probe/virtblk_remove > and see where the bad device came from. I attached the debug log using gdb on host. You can see the panic occurs at the function virtio_check_driver_offered_feature() who accesses the feature_table out-of range (actually, the size of array should be 8). But these data became wrong since bdev->bd_disk->private_data. And, could you tell me what data do you need me trace on virtblk_probe/virtblk_remove? I'll collect these in my environment. Thanks, Ren -- Best Regards -------------------------------------------------- Ren Mingxin Development Dept.I Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST) No.6 Wenzhu Road, Nanjing, 210012, China TEL: +86+25-86630566-8552 FUJITSU INTERNAL: 7998-8552 FAX: +86+25-83317685 MAIL:renmx@cn.fujitsu.com --------------------------------------------------