* [Qemu-devel] [PATCH 1.1 1/4] virtio-blk: report non-zero status when failing SG_IO requests
2012-05-16 10:54 [Qemu-devel] [PATCH 1.1 0/4] decouple VIRTIO_BLK_F_SCSI from SG_IO support Paolo Bonzini
@ 2012-05-16 10:54 ` Paolo Bonzini
2012-05-16 10:54 ` [Qemu-devel] [PATCH 1.1 2/4] virtio-blk: blockdev_mark_auto_del is transport-independent Paolo Bonzini
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2012-05-16 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mst
Linux really looks only at scsi->errors for SG_IO requests; it does
not look at the virtio request status at all. Because of this, when
a SG_IO request is failed early with virtio_blk_req_complete(req,
VIRTIO_BLK_S_UNSUPP), without writing hdr.status, it will look like
a success to the guest.
This is their bug, but we can make it safe for older guests now by
forcing scsi->errors to have a non-zero value whenever a request
has to be failed.
But if we fix the bug in the guest driver, we will have another problem
because QEMU returns VIRTIO_BLK_S_IOERR if the status is non-zero, and
Linux translates that to -EIO. Rather, the guest should succeed the
request and pass the non-zero status via the userspace-provided SG_IO
structure. So, remove the case where virtio_blk_handle_scsi can
return VIRTIO_BLK_S_IOERR.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/virtio-blk.c | 51 +++++++++++++++++++++++----------------------------
1 file changed, 23 insertions(+), 28 deletions(-)
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index d4bb400..7148572 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -145,20 +145,12 @@ static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s)
return req;
}
-#ifdef __linux__
static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
{
- struct sg_io_hdr hdr;
int ret;
- int status;
+ int status = VIRTIO_BLK_S_OK;
int i;
- if ((req->dev->vdev.guest_features & (1 << VIRTIO_BLK_F_SCSI)) == 0) {
- virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
- g_free(req);
- return;
- }
-
/*
* We require at least one output segment each for the virtio_blk_outhdr
* and the SCSI command block.
@@ -173,20 +165,26 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
}
/*
- * No support for bidirection commands yet.
+ * The scsi inhdr is placed in the second-to-last input segment, just
+ * before the regular inhdr.
*/
- if (req->elem.out_num > 2 && req->elem.in_num > 3) {
- virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
- g_free(req);
- return;
+ req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
+
+ if ((req->dev->vdev.guest_features & (1 << VIRTIO_BLK_F_SCSI)) == 0) {
+ status = VIRTIO_BLK_S_UNSUPP;
+ goto fail;
}
/*
- * The scsi inhdr is placed in the second-to-last input segment, just
- * before the regular inhdr.
+ * No support for bidirection commands yet.
*/
- req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
+ if (req->elem.out_num > 2 && req->elem.in_num > 3) {
+ status = VIRTIO_BLK_S_UNSUPP;
+ goto fail;
+ }
+#ifdef __linux__
+ struct sg_io_hdr hdr;
memset(&hdr, 0, sizeof(struct sg_io_hdr));
hdr.interface_id = 'S';
hdr.cmd_len = req->elem.out_sg[1].iov_len;
@@ -230,12 +228,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
ret = bdrv_ioctl(req->dev->bs, SG_IO, &hdr);
if (ret) {
status = VIRTIO_BLK_S_UNSUPP;
- hdr.status = ret;
- hdr.resid = hdr.dxfer_len;
- } else if (hdr.status) {
- status = VIRTIO_BLK_S_IOERR;
- } else {
- status = VIRTIO_BLK_S_OK;
+ goto fail;
}
/*
@@ -258,14 +251,16 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
virtio_blk_req_complete(req, status);
g_free(req);
-}
#else
-static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
-{
- virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
+ abort();
+#endif
+
+fail:
+ /* Just put anything nonzero so that the ioctl fails in the guest. */
+ stl_p(&req->scsi->errors, 255);
+ virtio_blk_req_complete(req, status);
g_free(req);
}
-#endif /* __linux__ */
typedef struct MultiReqBuffer {
BlockRequest blkreq[32];
--
1.7.10.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] [PATCH 1.1 2/4] virtio-blk: blockdev_mark_auto_del is transport-independent
2012-05-16 10:54 [Qemu-devel] [PATCH 1.1 0/4] decouple VIRTIO_BLK_F_SCSI from SG_IO support Paolo Bonzini
2012-05-16 10:54 ` [Qemu-devel] [PATCH 1.1 1/4] virtio-blk: report non-zero status when failing SG_IO requests Paolo Bonzini
@ 2012-05-16 10:54 ` Paolo Bonzini
2012-05-16 10:54 ` [Qemu-devel] [PATCH 1.1 3/4] virtio-blk: define VirtIOBlkConf Paolo Bonzini
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2012-05-16 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mst
Move it from virtio_blk_exit_pci to virtio_blk_exit.
This is included here because the next patch removes proxy->block.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/virtio-blk.c | 1 +
hw/virtio-pci.c | 1 -
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 7148572..0569382 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -626,5 +626,6 @@ void virtio_blk_exit(VirtIODevice *vdev)
{
VirtIOBlock *s = to_virtio_blk(vdev);
unregister_savevm(s->qdev, "virtio-blk", s);
+ blockdev_mark_auto_del(s->bs);
virtio_cleanup(vdev);
}
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 4a4413d..7b2d576 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -726,7 +726,6 @@ static int virtio_blk_exit_pci(PCIDevice *pci_dev)
virtio_pci_stop_ioeventfd(proxy);
virtio_blk_exit(proxy->vdev);
- blockdev_mark_auto_del(proxy->block.bs);
return virtio_exit_pci(pci_dev);
}
--
1.7.10.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] [PATCH 1.1 3/4] virtio-blk: define VirtIOBlkConf
2012-05-16 10:54 [Qemu-devel] [PATCH 1.1 0/4] decouple VIRTIO_BLK_F_SCSI from SG_IO support Paolo Bonzini
2012-05-16 10:54 ` [Qemu-devel] [PATCH 1.1 1/4] virtio-blk: report non-zero status when failing SG_IO requests Paolo Bonzini
2012-05-16 10:54 ` [Qemu-devel] [PATCH 1.1 2/4] virtio-blk: blockdev_mark_auto_del is transport-independent Paolo Bonzini
@ 2012-05-16 10:54 ` Paolo Bonzini
2012-05-16 10:54 ` [Qemu-devel] [PATCH 1.1 4/4] virtio-blk: always enable VIRTIO_BLK_F_SCSI Paolo Bonzini
2012-05-21 15:56 ` [Qemu-devel] [PATCH 1.1 0/4] decouple VIRTIO_BLK_F_SCSI from SG_IO support Paolo Bonzini
4 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2012-05-16 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mst
We will have to add another field to the virtio-blk configuration in
the next patch. Avoid a proliferation of arguments to virtio_blk_init.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/s390-virtio-bus.c | 7 +++----
hw/s390-virtio-bus.h | 4 ++--
hw/virtio-blk.c | 27 +++++++++++++--------------
hw/virtio-blk.h | 7 +++++++
hw/virtio-pci.c | 7 +++----
hw/virtio-pci.h | 4 ++--
hw/virtio.h | 4 ++--
7 files changed, 32 insertions(+), 28 deletions(-)
diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index 63ccd5c..43647a7 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -163,8 +163,7 @@ static int s390_virtio_blk_init(VirtIOS390Device *dev)
{
VirtIODevice *vdev;
- vdev = virtio_blk_init((DeviceState *)dev, &dev->block,
- &dev->block_serial);
+ vdev = virtio_blk_init((DeviceState *)dev, &dev->blk);
if (!vdev) {
return -1;
}
@@ -400,8 +399,8 @@ static TypeInfo s390_virtio_net = {
};
static Property s390_virtio_blk_properties[] = {
- DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, block),
- DEFINE_PROP_STRING("serial", VirtIOS390Device, block_serial),
+ DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, blk.conf),
+ DEFINE_PROP_STRING("serial", VirtIOS390Device, blk.serial),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h
index 49e6c46..4b99d02 100644
--- a/hw/s390-virtio-bus.h
+++ b/hw/s390-virtio-bus.h
@@ -17,6 +17,7 @@
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
+#include "virtio-blk.h"
#include "virtio-net.h"
#include "virtio-serial.h"
#include "virtio-scsi.h"
@@ -64,8 +65,7 @@ struct VirtIOS390Device {
ram_addr_t feat_offs;
uint8_t feat_len;
VirtIODevice *vdev;
- BlockConf block;
- char *block_serial;
+ VirtIOBlkConf blk;
NICConf nic;
uint32_t host_features;
virtio_serial_conf serial;
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 0569382..b46ecb3 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -29,7 +29,7 @@ typedef struct VirtIOBlock
void *rq;
QEMUBH *bh;
BlockConf *conf;
- char *serial;
+ VirtIOBlkConf *blk;
unsigned short sector_mask;
DeviceState *qdev;
} VirtIOBlock;
@@ -389,7 +389,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
* terminated by '\0' only when shorter than buffer.
*/
strncpy(req->elem.in_sg[0].iov_base,
- s->serial ? s->serial : "",
+ s->blk->serial ? s->blk->serial : "",
MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
g_free(req);
@@ -568,28 +568,27 @@ static const BlockDevOps virtio_block_ops = {
.resize_cb = virtio_blk_resize,
};
-VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf,
- char **serial)
+VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
{
VirtIOBlock *s;
int cylinders, heads, secs;
static int virtio_blk_id;
DriveInfo *dinfo;
- if (!conf->bs) {
+ if (!blk->conf.bs) {
error_report("drive property not set");
return NULL;
}
- if (!bdrv_is_inserted(conf->bs)) {
+ if (!bdrv_is_inserted(blk->conf.bs)) {
error_report("Device needs media, but drive is empty");
return NULL;
}
- if (!*serial) {
+ if (!blk->serial) {
/* try to fall back to value set with legacy -drive serial=... */
- dinfo = drive_get_by_blockdev(conf->bs);
+ dinfo = drive_get_by_blockdev(blk->conf.bs);
if (*dinfo->serial) {
- *serial = strdup(dinfo->serial);
+ blk->serial = strdup(dinfo->serial);
}
}
@@ -600,9 +599,9 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf,
s->vdev.get_config = virtio_blk_update_config;
s->vdev.get_features = virtio_blk_get_features;
s->vdev.reset = virtio_blk_reset;
- s->bs = conf->bs;
- s->conf = conf;
- s->serial = *serial;
+ s->bs = blk->conf.bs;
+ s->conf = &blk->conf;
+ s->blk = blk;
s->rq = NULL;
s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
@@ -614,10 +613,10 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf,
register_savevm(dev, "virtio-blk", virtio_blk_id++, 2,
virtio_blk_save, virtio_blk_load, s);
bdrv_set_dev_ops(s->bs, &virtio_block_ops, s);
- bdrv_set_buffer_alignment(s->bs, conf->logical_block_size);
+ bdrv_set_buffer_alignment(s->bs, s->conf->logical_block_size);
bdrv_iostatus_enable(s->bs);
- add_boot_device_path(conf->bootindex, dev, "/disk@0,0");
+ add_boot_device_path(s->conf->bootindex, dev, "/disk@0,0");
return &s->vdev;
}
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index 244dce4..70564a1 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -97,6 +97,12 @@ struct virtio_scsi_inhdr
uint32_t residual;
};
+struct VirtIOBlkConf
+{
+ BlockConf conf;
+ char *serial;
+};
+
#ifdef __linux__
#define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
@@ -105,4 +111,5 @@ struct virtio_scsi_inhdr
#define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
#endif
+
#endif
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 7b2d576..ae3ddd8 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -697,8 +697,7 @@ static int virtio_blk_init_pci(PCIDevice *pci_dev)
proxy->class_code != PCI_CLASS_STORAGE_OTHER)
proxy->class_code = PCI_CLASS_STORAGE_SCSI;
- vdev = virtio_blk_init(&pci_dev->qdev, &proxy->block,
- &proxy->block_serial);
+ vdev = virtio_blk_init(&pci_dev->qdev, &proxy->blk);
if (!vdev) {
return -1;
}
@@ -813,8 +812,8 @@ static int virtio_balloon_exit_pci(PCIDevice *pci_dev)
static Property virtio_blk_properties[] = {
DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
- DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, block),
- DEFINE_PROP_STRING("serial", VirtIOPCIProxy, block_serial),
+ DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
+ DEFINE_PROP_STRING("serial", VirtIOPCIProxy, blk.serial),
DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
index e560428..889e59e 100644
--- a/hw/virtio-pci.h
+++ b/hw/virtio-pci.h
@@ -15,6 +15,7 @@
#ifndef QEMU_VIRTIO_PCI_H
#define QEMU_VIRTIO_PCI_H
+#include "virtio-blk.h"
#include "virtio-net.h"
#include "virtio-serial.h"
#include "virtio-scsi.h"
@@ -32,8 +33,7 @@ typedef struct {
uint32_t flags;
uint32_t class_code;
uint32_t nvectors;
- BlockConf block;
- char *block_serial;
+ VirtIOBlkConf blk;
NICConf nic;
uint32_t host_features;
#ifdef CONFIG_LINUX
diff --git a/hw/virtio.h b/hw/virtio.h
index 0aef7d1..85aabe5 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -191,8 +191,8 @@ void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding,
void *opaque);
/* Base devices. */
-VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf,
- char **serial);
+typedef struct VirtIOBlkConf VirtIOBlkConf;
+VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk);
struct virtio_net_conf;
VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
struct virtio_net_conf *net);
--
1.7.10.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] [PATCH 1.1 4/4] virtio-blk: always enable VIRTIO_BLK_F_SCSI
2012-05-16 10:54 [Qemu-devel] [PATCH 1.1 0/4] decouple VIRTIO_BLK_F_SCSI from SG_IO support Paolo Bonzini
` (2 preceding siblings ...)
2012-05-16 10:54 ` [Qemu-devel] [PATCH 1.1 3/4] virtio-blk: define VirtIOBlkConf Paolo Bonzini
@ 2012-05-16 10:54 ` Paolo Bonzini
2012-05-21 15:56 ` [Qemu-devel] [PATCH 1.1 0/4] decouple VIRTIO_BLK_F_SCSI from SG_IO support Paolo Bonzini
4 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2012-05-16 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mst
VIRTIO_BLK_F_SCSI is supposed to mean whether the host can *parse*
SCSI requests, not *execute* them. You could run QEMU with scsi=on
and a file-backed disk, and QEMU would fail all SCSI requests even
though it advertises VIRTIO_BLK_F_SCSI.
Because we need to do this to fix a migration compatibility problem
related to how QEMU is invoked by management, we must do this
unconditionally even on older machine types. This more or less assumes
that no one ever invoked QEMU with scsi=off.
Here is how testing goes:
- old QEMU, scsi=on -> new QEMU, scsi=on
- new QEMU, scsi=on -> old QEMU, scsi=on
- old QEMU, scsi=off -> new QEMU, scsi=on
- new QEMU, scsi=off -> old QEMU, scsi=on
ok (new QEMU has VIRTIO_BLK_F_SCSI, adding host features is fine)
- old QEMU, scsi=off -> new QEMU, scsi=off
ok (new QEMU has VIRTIO_BLK_F_SCSI, adding host features is fine)
- old QEMU, scsi=on -> new QEMU, scsi=off
ok, bug fixed
- new QEMU, scsi=on -> old QEMU, scsi=off
doesn't work (same as: old QEMU, scsi=on -> old QEMU, scsi=off)
- new QEMU, scsi=off -> old QEMU, scsi=off
broken by the patch
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/s390-virtio-bus.c | 3 +++
hw/virtio-blk.c | 3 ++-
hw/virtio-blk.h | 7 +------
hw/virtio-pci.c | 3 +++
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index 43647a7..1d38a8f 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -401,6 +401,9 @@ static TypeInfo s390_virtio_net = {
static Property s390_virtio_blk_properties[] = {
DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, blk.conf),
DEFINE_PROP_STRING("serial", VirtIOS390Device, blk.serial),
+#ifdef __linux__
+ DEFINE_PROP_BIT("scsi", VirtIOS390Device, blk.scsi, 0, true),
+#endif
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index b46ecb3..f9e1896 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -170,7 +170,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
*/
req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
- if ((req->dev->vdev.guest_features & (1 << VIRTIO_BLK_F_SCSI)) == 0) {
+ if (!req->dev->blk->scsi) {
status = VIRTIO_BLK_S_UNSUPP;
goto fail;
}
@@ -504,6 +504,7 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
features |= (1 << VIRTIO_BLK_F_GEOMETRY);
features |= (1 << VIRTIO_BLK_F_TOPOLOGY);
features |= (1 << VIRTIO_BLK_F_BLK_SIZE);
+ features |= (1 << VIRTIO_BLK_F_SCSI);
if (bdrv_enable_write_cache(s->bs))
features |= (1 << VIRTIO_BLK_F_WCACHE);
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index 70564a1..d785001 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -101,15 +101,10 @@ struct VirtIOBlkConf
{
BlockConf conf;
char *serial;
+ uint32_t scsi;
};
-#ifdef __linux__
-#define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
- DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
- DEFINE_PROP_BIT("scsi", _state, _field, VIRTIO_BLK_F_SCSI, true)
-#else
#define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
-#endif
#endif
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index ae3ddd8..7932392 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -814,6 +814,9 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
DEFINE_PROP_STRING("serial", VirtIOPCIProxy, blk.serial),
+#ifdef __linux__
+ DEFINE_PROP_BIT("scsi", VirtIOPCIProxy, blk.scsi, 0, true),
+#endif
DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
--
1.7.10.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH 1.1 0/4] decouple VIRTIO_BLK_F_SCSI from SG_IO support
2012-05-16 10:54 [Qemu-devel] [PATCH 1.1 0/4] decouple VIRTIO_BLK_F_SCSI from SG_IO support Paolo Bonzini
` (3 preceding siblings ...)
2012-05-16 10:54 ` [Qemu-devel] [PATCH 1.1 4/4] virtio-blk: always enable VIRTIO_BLK_F_SCSI Paolo Bonzini
@ 2012-05-21 15:56 ` Paolo Bonzini
2012-05-21 16:15 ` Anthony Liguori
4 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2012-05-21 15:56 UTC (permalink / raw)
To: aliguori; +Cc: qemu-devel, mst
Il 16/05/2012 12:54, Paolo Bonzini ha scritto:
> Previous versions of these patches have been posted already, but they
> were lost. Sorry for realizing this quite late.
>
> VIRTIO_BLK_F_SCSI is supposed to mean whether the host can *parse*
> SCSI requests, not *execute* them. So it should always be enabled,
> and the scsi=on/off property tied to a separate configuration variable
> that is not guest visible.
>
> With this change, Linux has problems understanding failed requests, so
> patch 1 works around the Linux bugs.
>
> Important: because we need to do this to fix a migration compatibility
> problem when QEMU might be invoked with an old machine type, we must do
> this unconditionally. This more or less assumes that no one ever invoked
> QEMU with scsi=off, as it breaks migration from new QEMU, scsi=off to
> old QEMU, also scsi=off. However new->old is not supported upstream.
>
> S390 compile-tested only.
>
> Paolo Bonzini (4):
> virtio-blk: report non-zero status when failing SG_IO requests
> virtio-blk: blockdev_mark_auto_del is transport-independent
> virtio-blk: define VirtIOBlkConf
> virtio-blk: always enable VIRTIO_BLK_F_SCSI
>
> hw/s390-virtio-bus.c | 10 ++++---
> hw/s390-virtio-bus.h | 4 +--
> hw/virtio-blk.c | 80 ++++++++++++++++++++++++--------------------------
> hw/virtio-blk.h | 14 +++++----
> hw/virtio-pci.c | 11 +++----
> hw/virtio-pci.h | 4 +--
> hw/virtio.h | 4 +--
> 7 files changed, 64 insertions(+), 63 deletions(-)
>
Ping?
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH 1.1 0/4] decouple VIRTIO_BLK_F_SCSI from SG_IO support
2012-05-21 15:56 ` [Qemu-devel] [PATCH 1.1 0/4] decouple VIRTIO_BLK_F_SCSI from SG_IO support Paolo Bonzini
@ 2012-05-21 16:15 ` Anthony Liguori
0 siblings, 0 replies; 7+ messages in thread
From: Anthony Liguori @ 2012-05-21 16:15 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, mst
On 05/21/2012 10:56 AM, Paolo Bonzini wrote:
> Il 16/05/2012 12:54, Paolo Bonzini ha scritto:
>> Previous versions of these patches have been posted already, but they
>> were lost. Sorry for realizing this quite late.
>>
>> VIRTIO_BLK_F_SCSI is supposed to mean whether the host can *parse*
>> SCSI requests, not *execute* them. So it should always be enabled,
>> and the scsi=on/off property tied to a separate configuration variable
>> that is not guest visible.
>>
>> With this change, Linux has problems understanding failed requests, so
>> patch 1 works around the Linux bugs.
>>
>> Important: because we need to do this to fix a migration compatibility
>> problem when QEMU might be invoked with an old machine type, we must do
>> this unconditionally. This more or less assumes that no one ever invoked
>> QEMU with scsi=off, as it breaks migration from new QEMU, scsi=off to
>> old QEMU, also scsi=off. However new->old is not supported upstream.
>>
>> S390 compile-tested only.
>>
>> Paolo Bonzini (4):
>> virtio-blk: report non-zero status when failing SG_IO requests
>> virtio-blk: blockdev_mark_auto_del is transport-independent
>> virtio-blk: define VirtIOBlkConf
>> virtio-blk: always enable VIRTIO_BLK_F_SCSI
>>
>> hw/s390-virtio-bus.c | 10 ++++---
>> hw/s390-virtio-bus.h | 4 +--
>> hw/virtio-blk.c | 80 ++++++++++++++++++++++++--------------------------
>> hw/virtio-blk.h | 14 +++++----
>> hw/virtio-pci.c | 11 +++----
>> hw/virtio-pci.h | 4 +--
>> hw/virtio.h | 4 +--
>> 7 files changed, 64 insertions(+), 63 deletions(-)
>
>
> Ping?
Ack. I'll queue it for -rc3.
Regards,
Anthony Liguori
>
^ permalink raw reply [flat|nested] 7+ messages in thread