From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:52027)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1V19e8-00072e-0Y
	for qemu-devel@nongnu.org; Mon, 22 Jul 2013 02:31:01 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1V19e5-0003OL-3V
	for qemu-devel@nongnu.org; Mon, 22 Jul 2013 02:30:59 -0400
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
Message-ID: <51ECD190.60700@redhat.com>
Date: Mon, 22 Jul 2013 08:30:40 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1374401042-18511-1-git-send-email-aik@ozlabs.ru>
	<1374442491.3916.2.camel@pasglop> <51EC7AC9.3080306@ozlabs.ru>
	<1374452612.3916.17.camel@pasglop> <51EC8381.4020801@ozlabs.ru>
	<1374455044.3916.27.camel@pasglop>
In-Reply-To: <1374455044.3916.27.camel@pasglop>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [RFC PATCH] spapr-vscsi: add task management
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Anthony Liguori <aliguori@us.ibm.com>, Alexey Kardashevskiy <aik@ozlabs.ru>, Alexander Graf <agraf@suse.de>, qemu-devel@nongnu.org, Paul Mackerras <paulus@samba.org>, qemu-ppc@nongnu.org, David Gibson <david@gibson.dropbear.id.au>

Il 22/07/2013 03:04, Benjamin Herrenschmidt ha scritto:
> The free_request callback
> is not directly "linked" to task management though it happens to only
> be called in that case. It's a pre-requisite to task management, make it
> a separate patch.

Ben is right, in fact I'm not sure why you need free_request at all
(since vscsi_put_req is called from both vscsi_command_complete and
vscsi_request_cancelled).

free_request is only needed by hw/usb/dev-uas.c because it does
scsi_req_ref/scsi_req_unref in the HBA code.  There is no need for this,
AFAIK, in the current spapr_vscsi code; if it is done in the
reorganization patches, the free_request callback should be added there.

This part looks weird:

>      default:
>          vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x20, 0);
>          vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);

I suggest you check in the relevant part of the vscsi spec how you are
supposed to send back errors ("FUNCTION REJECTED", "INCORRECT LUN").
Once you do that, implementing the "query" TMFs is trivial.

Paolo