virtio-scsi: Fix acquire/release in dataplane handlers
After the AioContext lock push down, there is a race between virtio_scsi_dataplane_start and those "assert(s->ctx && s->dataplane_started)", because the latter doesn't isn't wrapped in aio_context_acquire. Reproducer is simply booting a Fedora guest with an empty virtio-scsi-dataplane controller: qemu-system-x86_64 \ -drive if=none,id=root,format=raw,file=Fedora-Cloud-Base-25-1.3.x86_64.raw \ -device virtio-scsi \ -device scsi-disk,drive=root,bootindex=1 \ -object iothread,id=io \ -device virtio-scsi-pci,iothread=io \ -net user,hostfwd=tcp::10022-:22 -net nic,model=virtio -m 2048 \ --enable-kvm Fix this by moving acquire/release pairs from virtio_scsi_handle_*_vq to their callers - and wrap the broken assertions in. Signed-off-by: Fam Zheng <famz@redhat.com> Message-Id: <20170317061447.16243-3-famz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
parent
3d69f82161
commit
7140778605
@ -52,28 +52,40 @@ void virtio_scsi_dataplane_setup(VirtIOSCSI *s, Error **errp)
|
|||||||
static bool virtio_scsi_data_plane_handle_cmd(VirtIODevice *vdev,
|
static bool virtio_scsi_data_plane_handle_cmd(VirtIODevice *vdev,
|
||||||
VirtQueue *vq)
|
VirtQueue *vq)
|
||||||
{
|
{
|
||||||
VirtIOSCSI *s = (VirtIOSCSI *)vdev;
|
bool progress;
|
||||||
|
VirtIOSCSI *s = VIRTIO_SCSI(vdev);
|
||||||
|
|
||||||
|
virtio_scsi_acquire(s);
|
||||||
assert(s->ctx && s->dataplane_started);
|
assert(s->ctx && s->dataplane_started);
|
||||||
return virtio_scsi_handle_cmd_vq(s, vq);
|
progress = virtio_scsi_handle_cmd_vq(s, vq);
|
||||||
|
virtio_scsi_release(s);
|
||||||
|
return progress;
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool virtio_scsi_data_plane_handle_ctrl(VirtIODevice *vdev,
|
static bool virtio_scsi_data_plane_handle_ctrl(VirtIODevice *vdev,
|
||||||
VirtQueue *vq)
|
VirtQueue *vq)
|
||||||
{
|
{
|
||||||
|
bool progress;
|
||||||
VirtIOSCSI *s = VIRTIO_SCSI(vdev);
|
VirtIOSCSI *s = VIRTIO_SCSI(vdev);
|
||||||
|
|
||||||
|
virtio_scsi_acquire(s);
|
||||||
assert(s->ctx && s->dataplane_started);
|
assert(s->ctx && s->dataplane_started);
|
||||||
return virtio_scsi_handle_ctrl_vq(s, vq);
|
progress = virtio_scsi_handle_ctrl_vq(s, vq);
|
||||||
|
virtio_scsi_release(s);
|
||||||
|
return progress;
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool virtio_scsi_data_plane_handle_event(VirtIODevice *vdev,
|
static bool virtio_scsi_data_plane_handle_event(VirtIODevice *vdev,
|
||||||
VirtQueue *vq)
|
VirtQueue *vq)
|
||||||
{
|
{
|
||||||
|
bool progress;
|
||||||
VirtIOSCSI *s = VIRTIO_SCSI(vdev);
|
VirtIOSCSI *s = VIRTIO_SCSI(vdev);
|
||||||
|
|
||||||
|
virtio_scsi_acquire(s);
|
||||||
assert(s->ctx && s->dataplane_started);
|
assert(s->ctx && s->dataplane_started);
|
||||||
return virtio_scsi_handle_event_vq(s, vq);
|
progress = virtio_scsi_handle_event_vq(s, vq);
|
||||||
|
virtio_scsi_release(s);
|
||||||
|
return progress;
|
||||||
}
|
}
|
||||||
|
|
||||||
static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n,
|
static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n,
|
||||||
|
@ -427,12 +427,10 @@ bool virtio_scsi_handle_ctrl_vq(VirtIOSCSI *s, VirtQueue *vq)
|
|||||||
VirtIOSCSIReq *req;
|
VirtIOSCSIReq *req;
|
||||||
bool progress = false;
|
bool progress = false;
|
||||||
|
|
||||||
virtio_scsi_acquire(s);
|
|
||||||
while ((req = virtio_scsi_pop_req(s, vq))) {
|
while ((req = virtio_scsi_pop_req(s, vq))) {
|
||||||
progress = true;
|
progress = true;
|
||||||
virtio_scsi_handle_ctrl_req(s, req);
|
virtio_scsi_handle_ctrl_req(s, req);
|
||||||
}
|
}
|
||||||
virtio_scsi_release(s);
|
|
||||||
return progress;
|
return progress;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -446,7 +444,9 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
virtio_scsi_acquire(s);
|
||||||
virtio_scsi_handle_ctrl_vq(s, vq);
|
virtio_scsi_handle_ctrl_vq(s, vq);
|
||||||
|
virtio_scsi_release(s);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req)
|
static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req)
|
||||||
@ -590,7 +590,6 @@ bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq)
|
|||||||
|
|
||||||
QTAILQ_HEAD(, VirtIOSCSIReq) reqs = QTAILQ_HEAD_INITIALIZER(reqs);
|
QTAILQ_HEAD(, VirtIOSCSIReq) reqs = QTAILQ_HEAD_INITIALIZER(reqs);
|
||||||
|
|
||||||
virtio_scsi_acquire(s);
|
|
||||||
do {
|
do {
|
||||||
virtio_queue_set_notification(vq, 0);
|
virtio_queue_set_notification(vq, 0);
|
||||||
|
|
||||||
@ -618,7 +617,6 @@ bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq)
|
|||||||
QTAILQ_FOREACH_SAFE(req, &reqs, next, next) {
|
QTAILQ_FOREACH_SAFE(req, &reqs, next, next) {
|
||||||
virtio_scsi_handle_cmd_req_submit(s, req);
|
virtio_scsi_handle_cmd_req_submit(s, req);
|
||||||
}
|
}
|
||||||
virtio_scsi_release(s);
|
|
||||||
return progress;
|
return progress;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -633,7 +631,9 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
virtio_scsi_acquire(s);
|
||||||
virtio_scsi_handle_cmd_vq(s, vq);
|
virtio_scsi_handle_cmd_vq(s, vq);
|
||||||
|
virtio_scsi_release(s);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void virtio_scsi_get_config(VirtIODevice *vdev,
|
static void virtio_scsi_get_config(VirtIODevice *vdev,
|
||||||
@ -709,12 +709,10 @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
virtio_scsi_acquire(s);
|
|
||||||
|
|
||||||
req = virtio_scsi_pop_req(s, vs->event_vq);
|
req = virtio_scsi_pop_req(s, vs->event_vq);
|
||||||
if (!req) {
|
if (!req) {
|
||||||
s->events_dropped = true;
|
s->events_dropped = true;
|
||||||
goto out;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (s->events_dropped) {
|
if (s->events_dropped) {
|
||||||
@ -724,7 +722,7 @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
|
|||||||
|
|
||||||
if (virtio_scsi_parse_req(req, 0, sizeof(VirtIOSCSIEvent))) {
|
if (virtio_scsi_parse_req(req, 0, sizeof(VirtIOSCSIEvent))) {
|
||||||
virtio_scsi_bad_req(req);
|
virtio_scsi_bad_req(req);
|
||||||
goto out;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
evt = &req->resp.event;
|
evt = &req->resp.event;
|
||||||
@ -744,19 +742,14 @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
|
|||||||
evt->lun[3] = dev->lun & 0xFF;
|
evt->lun[3] = dev->lun & 0xFF;
|
||||||
}
|
}
|
||||||
virtio_scsi_complete_req(req);
|
virtio_scsi_complete_req(req);
|
||||||
out:
|
|
||||||
virtio_scsi_release(s);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
bool virtio_scsi_handle_event_vq(VirtIOSCSI *s, VirtQueue *vq)
|
bool virtio_scsi_handle_event_vq(VirtIOSCSI *s, VirtQueue *vq)
|
||||||
{
|
{
|
||||||
virtio_scsi_acquire(s);
|
|
||||||
if (s->events_dropped) {
|
if (s->events_dropped) {
|
||||||
virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0);
|
virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0);
|
||||||
virtio_scsi_release(s);
|
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
virtio_scsi_release(s);
|
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -770,7 +763,9 @@ static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq)
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
virtio_scsi_acquire(s);
|
||||||
virtio_scsi_handle_event_vq(s, vq);
|
virtio_scsi_handle_event_vq(s, vq);
|
||||||
|
virtio_scsi_release(s);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense)
|
static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense)
|
||||||
@ -780,8 +775,10 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense)
|
|||||||
|
|
||||||
if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_CHANGE) &&
|
if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_CHANGE) &&
|
||||||
dev->type != TYPE_ROM) {
|
dev->type != TYPE_ROM) {
|
||||||
|
virtio_scsi_acquire(s);
|
||||||
virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE,
|
virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE,
|
||||||
sense.asc | (sense.ascq << 8));
|
sense.asc | (sense.ascq << 8));
|
||||||
|
virtio_scsi_release(s);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -803,9 +800,11 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
|
if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
|
||||||
|
virtio_scsi_acquire(s);
|
||||||
virtio_scsi_push_event(s, sd,
|
virtio_scsi_push_event(s, sd,
|
||||||
VIRTIO_SCSI_T_TRANSPORT_RESET,
|
VIRTIO_SCSI_T_TRANSPORT_RESET,
|
||||||
VIRTIO_SCSI_EVT_RESET_RESCAN);
|
VIRTIO_SCSI_EVT_RESET_RESCAN);
|
||||||
|
virtio_scsi_release(s);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -817,9 +816,11 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev,
|
|||||||
SCSIDevice *sd = SCSI_DEVICE(dev);
|
SCSIDevice *sd = SCSI_DEVICE(dev);
|
||||||
|
|
||||||
if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
|
if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
|
||||||
|
virtio_scsi_acquire(s);
|
||||||
virtio_scsi_push_event(s, sd,
|
virtio_scsi_push_event(s, sd,
|
||||||
VIRTIO_SCSI_T_TRANSPORT_RESET,
|
VIRTIO_SCSI_T_TRANSPORT_RESET,
|
||||||
VIRTIO_SCSI_EVT_RESET_REMOVED);
|
VIRTIO_SCSI_EVT_RESET_REMOVED);
|
||||||
|
virtio_scsi_release(s);
|
||||||
}
|
}
|
||||||
|
|
||||||
qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
|
qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
|
||||||
|
Loading…
Reference in New Issue
Block a user