From: Rusty Russell <rusty@rustcorp.com.au>
To: Heinz Graalfs <graalfs@linux.vnet.ibm.com>,
mst@redhat.com, virtualization@lists.linux-foundation.org
Cc: borntraeger@de.ibm.com
Subject: Re: [PATCH v3 RFC 2/4] virtio_blk: avoid further request queueing on device loss
Date: Wed, 04 Dec 2013 14:34:01 +1030 [thread overview]
Message-ID: <87pppdw9ce.fsf@rustcorp.com.au> (raw)
In-Reply-To: <1385548360-31943-3-git-send-email-graalfs@linux.vnet.ibm.com>
Heinz Graalfs <graalfs@linux.vnet.ibm.com> writes:
> Code is added to the remove callback to verify if a device was lost.
>
> In case of a device loss further request queueing should be prevented
> by setting appropriate queue flags prior to invoking del_gendisk().
> Blocking of request queueing leads to appropriate I/O errors when data
> are tried to be synched. Trying to synch data to a lost block device
> doesn't make too much sense.
Sure, but this isn't the only case where we should set these flags,
right? I would think if any virtqueue is reported broken, we should
mark the queue dying too.
> If the current remove callback was triggered due to an unregister driver,
> and the surprize_removal is not already set (although the actual device
> is already gone, e.g. virsh detach), del_gendisk() would be triggered
> resulting in a hang. This is a weird situation, and most likely not
> 'serializable'.
This seems like abusing the vdev struct to pass an argument to
virtblk_remove().
How about you mark all virtqueues broken in the "unexpected removal"
case? Then the driver should correctly fail to deliver requests.
Cheers,
Rusty.
next prev parent reply other threads:[~2013-12-04 4:04 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-27 10:32 [PATCH v3 RFC 0/4] virtio: add 'surprize_removal' to virtio_device Heinz Graalfs
2013-11-27 10:32 ` [PATCH v3 RFC 1/4] virtio: add surprize_removal " Heinz Graalfs
2013-11-27 10:32 ` [PATCH v3 RFC 2/4] virtio_blk: avoid further request queueing on device loss Heinz Graalfs
2013-12-04 4:04 ` Rusty Russell [this message]
2013-11-27 10:32 ` [PATCH v3 RFC 3/4] virtio_blk: avoid calling blk_cleanup_queue() " Heinz Graalfs
2013-11-27 10:47 ` Michael S. Tsirkin
2013-11-27 11:37 ` Heinz Graalfs
2013-11-27 12:28 ` Michael S. Tsirkin
2013-11-27 12:49 ` Michael S. Tsirkin
2013-11-27 14:15 ` Heinz Graalfs
2013-11-27 14:37 ` Michael S. Tsirkin
2013-11-27 10:32 ` [PATCH v3 RFC 4/4] virtio_ccw: set surprize_removal in virtio_device if a device was lost Heinz Graalfs
2013-11-27 10:49 ` Michael S. Tsirkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87pppdw9ce.fsf@rustcorp.com.au \
--to=rusty@rustcorp.com.au \
--cc=borntraeger@de.ibm.com \
--cc=graalfs@linux.vnet.ibm.com \
--cc=mst@redhat.com \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.