| Message ID | 1278418136-24556-7-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jul 06, 2010 at 02:08:49PM +0200, Markus Armbruster wrote: > drive_init() doesn't permit rerror for if=scsi, but that's worthless: > we get it via if=none and -device. > > Moreover, scsi-generic doesn't support werror. Since drive_init() > doesn't catch that, option werror was silently ignored even with > if=scsi. > > Wart: unlike drive_init(), we don't reject the default action when > it's explicitly specified. That's because we can't distinguish "no > rerror option" from "rerror=report", or "no werror" from > "rerror=enospc". Left for another day. I can't see a good reason that scsi doesn't support the rerror option, and implementing is trivial. So while this patch looks correct I'd rather see rerror implemented for scsi than hacking around the lack of it.
Christoph Hellwig <hch@lst.de> writes: > On Tue, Jul 06, 2010 at 02:08:49PM +0200, Markus Armbruster wrote: >> drive_init() doesn't permit rerror for if=scsi, but that's worthless: >> we get it via if=none and -device. >> >> Moreover, scsi-generic doesn't support werror. Since drive_init() >> doesn't catch that, option werror was silently ignored even with >> if=scsi. >> >> Wart: unlike drive_init(), we don't reject the default action when >> it's explicitly specified. That's because we can't distinguish "no >> rerror option" from "rerror=report", or "no werror" from >> "rerror=enospc". Left for another day. > > I can't see a good reason that scsi doesn't support the rerror option, > and implementing is trivial. So while this patch looks correct I'd > rather see rerror implemented for scsi than hacking around the lack of > it. You got a point there. Same for fdc. However, this is the best *I* can do in time for .13. Let's add the missin error action support after the release, okay?
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 3e41011..c30709c 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1059,6 +1059,11 @@ static int scsi_disk_initfn(SCSIDevice *dev) s->bs = s->qdev.conf.bs; is_cd = bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM; + if (bdrv_get_on_error(s->bs, 1) != BLOCK_ERR_REPORT) { + error_report("Device doesn't support drive option rerror"); + return -1; + } + if (!s->serial) { /* try to fall back to value set with legacy -drive serial=... */ dinfo = drive_get_by_blockdev(s->bs); diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c index 3915e78..a8b4176 100644 --- a/hw/scsi-generic.c +++ b/hw/scsi-generic.c @@ -474,6 +474,15 @@ static int scsi_generic_initfn(SCSIDevice *dev) return -1; } + if (bdrv_get_on_error(s->bs, 0) != BLOCK_ERR_STOP_ENOSPC) { + error_report("Device doesn't support drive option werror"); + return -1; + } + if (bdrv_get_on_error(s->bs, 1) != BLOCK_ERR_REPORT) { + error_report("Device doesn't support drive option rerror"); + return -1; + } + /* check we are using a driver managing SG_IO (version 3 and after */ if (bdrv_ioctl(s->bs, SG_GET_VERSION_NUM, &sg_version) < 0 || sg_version < 30000) {
drive_init() doesn't permit rerror for if=scsi, but that's worthless: we get it via if=none and -device. Moreover, scsi-generic doesn't support werror. Since drive_init() doesn't catch that, option werror was silently ignored even with if=scsi. Wart: unlike drive_init(), we don't reject the default action when it's explicitly specified. That's because we can't distinguish "no rerror option" from "rerror=report", or "no werror" from "rerror=enospc". Left for another day. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/scsi-disk.c | 5 +++++ hw/scsi-generic.c | 9 +++++++++ 2 files changed, 14 insertions(+), 0 deletions(-)