Re: [PATCH v2] scsi: target: alua: do not report emtpy port group

From: Mike Christie <michael.christie@oracle.com>
To: Dmitry Bogdanov <d.bogdanov@yadro.com>,
	Martin Petersen <martin.petersen@oracle.com>,
	target-devel@vger.kernel.org
Cc: Bart Van Assche <bvanassche@acm.org>,
	linux-scsi@vger.kernel.org, linux@yadro.com
Subject: Re: [PATCH v2] scsi: target: alua: do not report emtpy port group
Date: Mon, 12 Sep 2022 12:49:22 -0500	[thread overview]
Message-ID: <92f6024a-7898-e281-099a-c79b67fecafe@oracle.com> (raw)
In-Reply-To: <20220912125457.22573-1-d.bogdanov@yadro.com>

On 9/12/22 7:54 AM, Dmitry Bogdanov wrote:
> Default target port group is always returned in the list of port
> groups, even if the behaviour is unwanted, i.e. it has no members and
> non-default port groups are primary port groups.
> 
> SPC-4 ("5.15.2.7 Target port asymmetric access state reporting")
> states that a target MAY not provide info about port groups that do not
> contain the current port through that the RTPG is received.
> 

Where is that? I see where it says the state value for a group might not
be up to date when the RTPG is sent through a different port. Are you
taking that to mean we don't have to report entire groups?

Note that I also don't see where it says we have to return every group.

Remember how ESX used to send a RTPG to one port and expect that it got
every group and that the state info was all in sync (basically opposite
if what's in the spec now)?

The spec and ESX were updated, but I don't know if other OSs did this and
if/when everyone was updated. Do you know this info? Are the old ESX versions
that worked like that end of life?

> This patch hides port groups with no ports in REPORT TARGET PORT GROUPS
> response.
> 
> Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
> ---
> v2:
>   new solution - just skip all empty groups
> ---
>  drivers/target/target_core_alua.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
> index fb91423a4e2e..c8470e7c0e10 100644
> --- a/drivers/target/target_core_alua.c
> +++ b/drivers/target/target_core_alua.c
> @@ -164,6 +164,9 @@ target_emulate_report_target_port_groups(struct se_cmd *cmd)
>  	spin_lock(&dev->t10_alua.tg_pt_gps_lock);
>  	list_for_each_entry(tg_pt_gp, &dev->t10_alua.tg_pt_gps_list,
>  			tg_pt_gp_list) {
> +		/* Skip empty port groups */
> +		if (!tg_pt_gp->tg_pt_gp_members)
> +			continue;
>  		/*
>  		 * Check if the Target port group and Target port descriptor list
>  		 * based on tg_pt_gp_members count will fit into the response payload.