From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59599) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T5wh9-0008AB-HW for qemu-devel@nongnu.org; Mon, 27 Aug 2012 06:37:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T5wh7-0004IK-PA for qemu-devel@nongnu.org; Mon, 27 Aug 2012 06:37:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:63286) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T5wh7-0004I8-Gy for qemu-devel@nongnu.org; Mon, 27 Aug 2012 06:37:21 -0400 Message-ID: <503B4DDB.5070702@redhat.com> Date: Mon, 27 Aug 2012 12:37:15 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1343700436-25315-1-git-send-email-ronniesahlberg@gmail.com> <1343700436-25315-2-git-send-email-ronniesahlberg@gmail.com> <501785F0.8050607@redhat.com> In-Reply-To: <501785F0.8050607@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ in sense codes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, Ronnie Sahlberg Am 31.07.2012 09:14, schrieb Paolo Bonzini: > Il 31/07/2012 04:07, Ronnie Sahlberg ha scritto: >> Add support for setting the ASCQ for SCSI sense codes in the ATAPI driver. >> Use this to set ASCQ==2 for the medium removal prevention that is recommended in MMC for this condition. >> >> asc:0x53 ascq:0x02 is the recommended error for MEDIUM_REMOVAL_PREVENTED and is listed in Annex F in MMC > > You also need to cover migration. > > You could either add a subsection, or something like this: > > diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c > index f7f714c..89c0157 100644 > --- a/hw/ide/atapi.c > +++ b/hw/ide/atapi.c > @@ -1143,3 +1143,20 @@ void ide_atapi_cmd(IDEState *s) > > ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_ILLEGAL_OPCODE); > } > + > +void ide_atapi_post_load(IDEState *s, int version_id) > +{ > + if (version_id < 3) { > + if (s->sense_key == UNIT_ATTENTION && > + s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) { > + s->cdrom_changed = 1; > + } > + } > + > + /* This is simpler than adding a subsection just for the ascq. */ > + if (s->asc == ASC_MEDIA_REMOVAL_PREVENTED) { > + s->ascq = 2; > + } else { > + s->ascq = 0; > + } > +} > diff --git a/hw/ide/core.c b/hw/ide/core.c > index cb5ca4b..959ac48 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -2154,12 +2154,7 @@ static int ide_drive_post_load(void *opaque, int version_id) > { > IDEState *s = opaque; > > - if (version_id < 3) { > - if (s->sense_key == UNIT_ATTENTION && > - s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) { > - s->cdrom_changed = 1; > - } > - } > + ide_atapi_post_load(s, version_id); > if (s->identify_set) { > bdrv_set_enable_write_cache(s->bs, !!(s->identify_data[85] & (1 << 5))); > } > diff --git a/hw/ide/internal.h b/hw/ide/internal.h > index 7170bd9..2572461 100644 > --- a/hw/ide/internal.h > +++ b/hw/ide/internal.h > @@ -572,6 +572,7 @@ BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs, > /* hw/ide/atapi.c */ > void ide_atapi_cmd(IDEState *s); > void ide_atapi_cmd_reply_end(IDEState *s); > +void ide_atapi_post_load(IDEState *s, int version_id); > > /* hw/ide/qdev.c */ > void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id); > > > In fact, I wonder if it is simpler to "make up" the ascq directly in > cmd_request_sense, instead of changing all invocations of > ide_atapi_cmd_error. Kevin, any preferences? Oh, I missed this question and wondered why there was no v2... I'd prefer this patch with migration support added. Kevin