| Message ID | 1331036527-7651-4-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Mar 06, 2012 at 01:22:07PM +0100, Paolo Bonzini wrote: > The guest must already be prepared to see SG_IO support > disappear from under its feet, for example if migration > refers to a block device on the source and file-based > storage on the destination; or more likely, if the source > kernel allows (gasp) SG_IO on a partition and the destination > does not. So, we can migrate safely even if the source > had VIRTIO_BLK_F_SCSI and the destination does not. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> My first reaction is you want a new non guest visible flag to control whether SG_IO fails on host. guest visible ones must be consistent across migration. > --- > hw/virtio-blk.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > index c95f8fc..9a4158a 100644 > --- a/hw/virtio-blk.c > +++ b/hw/virtio-blk.c > @@ -542,7 +542,7 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id) > if (version_id != 2) > return -EINVAL; > > - ret = virtio_load(&s->vdev, f, 0); > + ret = virtio_load(&s->vdev, f, VIRTIO_BLK_F_SCSI); > if (ret) { > return ret; > } > -- > 1.7.7.6
Il 06/03/2012 15:53, Michael S. Tsirkin ha scritto: > > The guest must already be prepared to see SG_IO support > > disappear from under its feet, for example if migration > > refers to a block device on the source and file-based > > storage on the destination; or more likely, if the source > > kernel allows (gasp) SG_IO on a partition and the destination > > does not. So, we can migrate safely even if the source > > had VIRTIO_BLK_F_SCSI and the destination does not. > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > My first reaction is you want a new non guest > visible flag to control whether SG_IO fails on host. > guest visible ones must be consistent across migration. So scsi=off/on would control VIRTIO_BLK_F_SCSI, while the new flag would cause requests to fail. Then it's simpler to do the other way round. Make the "new non guest-visible flag" be scsi=on/off and set VIRTIO_BLK_F_SCSI unconditionally as you suggested first. Paolo
On 03/06/2012 06:22 AM, Paolo Bonzini wrote: > The guest must already be prepared to see SG_IO support > disappear from under its feet, for example if migration > refers to a block device on the source and file-based > storage on the destination; or more likely, if the source > kernel allows (gasp) SG_IO on a partition and the destination > does not. So, we can migrate safely even if the source > had VIRTIO_BLK_F_SCSI and the destination does not. I don't know how comfortable I feel about this. You can't just remove a feature in flight. The guest is going to behave differently in such a way that the host isn't expecting. Yes, it should fail gracefully, but nonetheless it will fail. Aren't you just delaying the inevitable? Instead of having migration fail, the guest workload is going to fail. How is this an improvement? Regards, Anthony Liguorig > > Signed-off-by: Paolo Bonzini<pbonzini@redhat.com> > --- > hw/virtio-blk.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > index c95f8fc..9a4158a 100644 > --- a/hw/virtio-blk.c > +++ b/hw/virtio-blk.c > @@ -542,7 +542,7 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id) > if (version_id != 2) > return -EINVAL; > > - ret = virtio_load(&s->vdev, f, 0); > + ret = virtio_load(&s->vdev, f, VIRTIO_BLK_F_SCSI); > if (ret) { > return ret; > }
Il 06/03/2012 18:03, Anthony Liguori ha scritto: > I don't know how comfortable I feel about this. > > You can't just remove a feature in flight. The guest is going to behave > differently in such a way that the host isn't expecting. Yes, it should > fail gracefully, but nonetheless it will fail. > > Aren't you just delaying the inevitable? Instead of having migration > fail, the guest workload is going to fail. How is this an improvement? VIRTIO_BLK_F_SCSI feature was almost never used but was always marked as available. Because of possible security problems connected to it, libvirt started making it an opt-in feature. In practice, you need to configure your host specially if you want to use SCSI passthrough (e.g. you must not use labels and UUIDs in your /etc/fstab), so it's safe to assume that guests that have SG_IO disabled under their feet will keep working. That said, instead of this hack we can just decouple scsi=on/off from VIRTIO_BLK_F_SCSI, and just report the feature. After all we do not clear VIRTIO_BLK_F_SCSI just because the device is backed by a file or partition, yet SG_IO is still unavailable in those cases. I'll send patches for this tomorrow. Paolo
On 03/06/2012 11:12 AM, Paolo Bonzini wrote: > Il 06/03/2012 18:03, Anthony Liguori ha scritto: >> I don't know how comfortable I feel about this. >> >> You can't just remove a feature in flight. The guest is going to behave >> differently in such a way that the host isn't expecting. Yes, it should >> fail gracefully, but nonetheless it will fail. >> >> Aren't you just delaying the inevitable? Instead of having migration >> fail, the guest workload is going to fail. How is this an improvement? > > VIRTIO_BLK_F_SCSI feature was almost never used but was always marked as > available. Because of possible security problems connected to it, > libvirt started making it an opt-in feature. > > In practice, you need to configure your host specially if you want to > use SCSI passthrough (e.g. you must not use labels and UUIDs in your > /etc/fstab), so it's safe to assume that guests that have SG_IO disabled > under their feet will keep working. > > That said, instead of this hack we can just decouple scsi=on/off from > VIRTIO_BLK_F_SCSI, and just report the feature. After all we do not > clear VIRTIO_BLK_F_SCSI just because the device is backed by a file or > partition, yet SG_IO is still unavailable in those cases. I'll send > patches for this tomorrow. Okay, that makes more sense to me. I think this is an exceptional circumstance so handling it as a one-off verses a generic mechanism would be preferred. Regards, Anthony Liguori > > Paolo
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index c95f8fc..9a4158a 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -542,7 +542,7 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id) if (version_id != 2) return -EINVAL; - ret = virtio_load(&s->vdev, f, 0); + ret = virtio_load(&s->vdev, f, VIRTIO_BLK_F_SCSI); if (ret) { return ret; }
The guest must already be prepared to see SG_IO support disappear from under its feet, for example if migration refers to a block device on the source and file-based storage on the destination; or more likely, if the source kernel allows (gasp) SG_IO on a partition and the destination does not. So, we can migrate safely even if the source had VIRTIO_BLK_F_SCSI and the destination does not. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/virtio-blk.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)