From 5b3adfa31a46c27d9719bd67af4206a03904ca4d Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 13 Mar 2012 15:39:10 +0100
Subject: [PATCH] force-enable VIRTIO_BLK_F_SCSI if present on migration
source
RH-Author: Paolo Bonzini <pbonzini@redhat.com>
Message-id: <1331653150-28921-1-git-send-email-pbonzini@redhat.com>
Patchwork-id: 38490
O-Subject: [RHEL 6.3 qemu-kvm PATCH] force-enable VIRTIO_BLK_F_SCSI if present on migration source
Bugzilla: 800536
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
RH-Acked-by: Markus Armbruster <armbru@redhat.com>
RH-Acked-by: Orit Wasserman <owasserm@redhat.com>
Bugzilla: 800536
Upstream status: different patch needed there, see below.
Brew build: 4147215
After finding the kernel SG_IO vulnerability, libvirt switched SG_IO
support in virtio-blk from the QEMU default of "on" to opt-in; libvirt
now looks at domain XML and passes scsi=on/scsi=off explicitly.
Unfortunately this caused problems with migration. If the destination
uses a new libvirt, the same domain XML will have a different feature
set. In retrospect the right thing to do should have been to disable
SG_IO *in QEMU* for new machines, and have libvirt pass scsi=on only
(otherwise relying on the default).
Unfortunately this is now too late to change, and we need to deal with
the problem.
Upstream, mst asked to always turn on VIRTIO_BLK_F_SCSI, and reject
requests later. This is possible because the feature bit does not need
to represent "do SG_IO requests work?", only "are SG_IO requests
understood?". SG_IO requests can and will be rejected anyway if the
storage is a file or partition or LVM volume.
This is quite clean, but unfortunately it creates another migration
breakage: old QEMU with scsi=off will refuse migration from new QEMU
(also with scsi=off). Upstream does not support new->old migration,
but we do, so we need something else. This patch just overrides scsi=off
to scsi=on if the migration source had it that way.
Since we're deviating from upstream we need to consider what to do
for RHEL7. There are two possibilities:
1) RHEL7->RHEL6 migration will not be supported, and the upstream
solution will be okay for RHEL6->RHEL7 migration even with rhel6.x.0
machine types.
2) RHEL7->RHEL6 migration will be supported, and we'll have to
re-add the coupling between scsi=on/off and VIRTIO_BLK_F_SCSI for
the rhel6.x.0 machine type.
Tested here. After migrating a "scsi=on" guest to "scsi=off"
with the patch installed, migration works and "info qtree"
shows:
dev-prop: scsi = on
Please hold your nose closed, review and ack.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/virtio-blk.c | 12 +++++++++++-
hw/virtio-pci.c | 7 +++++++
hw/virtio.c | 15 ++++++++++++---
hw/virtio.h | 3 +++
4 files changed, 33 insertions(+), 4 deletions(-)
Signed-off-by: Michal Novotny <minovotn@redhat.com>
---
hw/virtio-blk.c | 13 ++++++++++++-
hw/virtio-pci.c | 7 +++++++
hw/virtio.c | 15 ++++++++++++---
hw/virtio.h | 3 +++
4 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 278e886..05209c6 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -545,7 +545,18 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id)
if (version_id != 2)
return -EINVAL;
- ret = virtio_load(&s->vdev, f);
+ /* RHEL only. Upstream we will fix this by ensuring that VIRTIO_BLK_F_SCSI
+ * is always set. This however will cause migration from new QEMU to old
+ * QEMU to fail if both have scsi=off. We cannot use compatibility
+ * properties because the scsi property is being overridden by management
+ * (libvirt).
+ *
+ * If RHEL7->RHEL6 migration will be supported, we'll have to disable
+ * VIRTIO_BLK_F_SCSI in the migration stream when running on a rhel6.x.0
+ * machine type with scsi=off. Otherwise SG_IO will magically start
+ * working on the destination.
+ */
+ ret = virtio_load_with_features(&s->vdev, f, 1 << VIRTIO_BLK_F_SCSI);
if (ret) {
return ret;
}
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 3e7bc71..b2a558d 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -557,6 +557,12 @@ static unsigned virtio_pci_get_features(void *opaque)
return proxy->host_features;
}
+static void virtio_pci_force_features(void *opaque, uint32_t extra_features)
+{
+ VirtIOPCIProxy *proxy = opaque;
+ proxy->host_features |= extra_features;
+}
+
static void virtio_pci_guest_notifier_read(void *opaque)
{
VirtQueue *vq = opaque;
@@ -743,6 +749,7 @@ static const VirtIOBindings virtio_pci_bindings = {
.save_queue = virtio_pci_save_queue,
.load_queue = virtio_pci_load_queue,
.get_features = virtio_pci_get_features,
+ .force_features = virtio_pci_force_features,
.query_guest_notifiers = virtio_pci_query_guest_notifiers,
.set_host_notifier = virtio_pci_set_host_notifier,
.set_guest_notifiers = virtio_pci_set_guest_notifiers,
diff --git a/hw/virtio.c b/hw/virtio.c
index 445db6e..c7f39f2 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -761,12 +761,12 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
}
}
-int virtio_load(VirtIODevice *vdev, QEMUFile *f)
+int virtio_load_with_features(VirtIODevice *vdev, QEMUFile *f,
+ uint32_t extra_features)
{
int num, i, ret;
uint32_t features;
- uint32_t supported_features =
- vdev->binding->get_features(vdev->binding_opaque);
+ uint32_t supported_features;
if (vdev->binding->load_config) {
ret = vdev->binding->load_config(vdev->binding_opaque, f);
@@ -778,6 +778,10 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
qemu_get_8s(f, &vdev->isr);
qemu_get_be16s(f, &vdev->queue_sel);
qemu_get_be32s(f, &features);
+
+ vdev->binding->force_features(vdev->binding_opaque,
+ features & extra_features);
+ supported_features = vdev->binding->get_features(vdev->binding_opaque);
if (features & ~supported_features) {
fprintf(stderr, "Features 0x%x unsupported. Allowed features: 0x%x\n",
features, supported_features);
@@ -812,6 +816,11 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
return 0;
}
+int virtio_load(VirtIODevice *vdev, QEMUFile *f)
+{
+ return virtio_load_with_features(vdev, f, 0);
+}
+
void virtio_cleanup(VirtIODevice *vdev)
{
qemu_del_vm_change_state_handler(vdev->vmstate);
diff --git a/hw/virtio.h b/hw/virtio.h
index 7ce62c9..d0ddb44 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -95,6 +95,7 @@ typedef struct {
int (*load_config)(void * opaque, QEMUFile *f);
int (*load_queue)(void * opaque, int n, QEMUFile *f);
unsigned (*get_features)(void * opaque);
+ void (*force_features)(void * opaque, uint32_t features);
bool (*query_guest_notifiers)(void * opaque);
int (*set_guest_notifiers)(void * opaque, bool assigned);
int (*set_host_notifier)(void * opaque, int n, bool assigned);
@@ -159,6 +160,8 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
void virtio_save(VirtIODevice *vdev, QEMUFile *f);
int virtio_load(VirtIODevice *vdev, QEMUFile *f);
+int virtio_load_with_features(VirtIODevice *vdev, QEMUFile *f,
+ uint32_t supported_features);
void virtio_cleanup(VirtIODevice *vdev);
--
1.7.7.6