scsi_debug: add implicit zones in max_open check

[PATCH] scsi_debug: add implicit zones in max_open check

Posted by Andrew Bernal 2 months, 2 weeks ago

`max_open` also needs to check implicit open, not just explicit
This is consistent with the logic in `zbc_open_zone`, on line 3809.

https://zonedstorage.io/docs/introduction/zoned-storage Open Zones limit
is defined as a "limit on the total number of zones that can simultaneously
be in an implicit open or explicit open state"

Signed-off-by: Andrew Bernal <andrewlbernal@gmail.com>
---
 drivers/scsi/scsi_debug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index aef33d1e346a..0edb9a4698ca 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3943,7 +3943,7 @@ static int check_zbc_access_params(struct scsi_cmnd *scp,
 	/* Handle implicit open of closed and empty zones */
 	if (zsp->z_cond == ZC1_EMPTY || zsp->z_cond == ZC4_CLOSED) {
 		if (devip->max_open &&
-		    devip->nr_exp_open >= devip->max_open) {
+		    devip->nr_imp_open + devip->nr_exp_open >= devip->max_open) {
 			mk_sense_buffer(scp, DATA_PROTECT,
 					INSUFF_RES_ASC,
 					INSUFF_ZONE_ASCQ);
@@ -6101,7 +6101,7 @@ static int resp_open_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	if (all) {
 		/* Check if all closed zones can be open */
 		if (devip->max_open &&
-		    devip->nr_exp_open + devip->nr_closed > devip->max_open) {
+		    devip->nr_imp_open + devip->nr_exp_open + devip->nr_closed > devip->max_open) {
 			mk_sense_buffer(scp, DATA_PROTECT, INSUFF_RES_ASC,
 					INSUFF_ZONE_ASCQ);
 			res = check_condition_result;
@@ -6136,7 +6136,7 @@ static int resp_open_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	if (zc == ZC3_EXPLICIT_OPEN || zc == ZC5_FULL)
 		goto fini;
 
-	if (devip->max_open && devip->nr_exp_open >= devip->max_open) {
+	if (devip->max_open && devip->nr_imp_open + devip->nr_exp_open >= devip->max_open) {
 		mk_sense_buffer(scp, DATA_PROTECT, INSUFF_RES_ASC,
 				INSUFF_ZONE_ASCQ);
 		res = check_condition_result;
-- 
2.49.0

Re: [PATCH] scsi_debug: add implicit zones in max_open check

Posted by Bart Van Assche 2 months, 1 week ago

On 7/22/25 1:32 PM, Andrew Bernal wrote:
> https://zonedstorage.io/docs/introduction/zoned-storage Open Zones limit
> is defined as a "limit on the total number of zones that can simultaneously
> be in an implicit open or explicit open state"

That's not an official standard and hence should not be used to motivate
this patch. Additionally, I don't see how a zone could be simultaneously 
in the implicit open and the explicit open state. According to the ZBC-2 
standard, these states are mutually exclusive.

devip->max_open is reported to the initiator in VPD page B6
as the MAXIMUM NUMBER OF OPEN SEQUENTIAL WRITE REQUIRED ZONES. From
ZBC-2 section 4.5.3.4.2: "If the value in the MAXIMUM NUMBER OF OPEN
SEQUENTIAL WRITE REQUIRED ZONES field (see 6.5.2) is non-zero and
the number of zones with a Zone Condition of EXPLICITLY OPENED is equal
to the value in the MAXIMUM NUMBER OF OPEN SEQUENTIAL WRITE REQUIRED
ZONES field, then a command that writes to or attempts to open a
sequential write required zone with a zone condition of EMPTY or CLOSED
is terminated with CHECK CONDITION status with sense key set to DATA
PROTECT and the additional sense code set to INSUFFICIENT ZONE RESOURCES
(see 4.5.3.2.8)."

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index aef33d1e346a..0edb9a4698ca 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -3943,7 +3943,7 @@ static int check_zbc_access_params(struct scsi_cmnd *scp,
>   	/* Handle implicit open of closed and empty zones */
>   	if (zsp->z_cond == ZC1_EMPTY || zsp->z_cond == ZC4_CLOSED) {
>   		if (devip->max_open &&
> -		    devip->nr_exp_open >= devip->max_open) {
> +		    devip->nr_imp_open + devip->nr_exp_open >= devip->max_open) {
>   			mk_sense_buffer(scp, DATA_PROTECT,
>   					INSUFF_RES_ASC,
>   					INSUFF_ZONE_ASCQ);
> @@ -6101,7 +6101,7 @@ static int resp_open_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
>   	if (all) {
>   		/* Check if all closed zones can be open */
>   		if (devip->max_open &&
> -		    devip->nr_exp_open + devip->nr_closed > devip->max_open) {
> +		    devip->nr_imp_open + devip->nr_exp_open + devip->nr_closed > devip->max_open) {
>   			mk_sense_buffer(scp, DATA_PROTECT, INSUFF_RES_ASC,
>   					INSUFF_ZONE_ASCQ);
>   			res = check_condition_result;
> @@ -6136,7 +6136,7 @@ static int resp_open_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
>   	if (zc == ZC3_EXPLICIT_OPEN || zc == ZC5_FULL)
>   		goto fini;
>   
> -	if (devip->max_open && devip->nr_exp_open >= devip->max_open) {
> +	if (devip->max_open && devip->nr_imp_open + devip->nr_exp_open >= devip->max_open) {
>   		mk_sense_buffer(scp, DATA_PROTECT, INSUFF_RES_ASC,
>   				INSUFF_ZONE_ASCQ);
>   		res = check_condition_result;

Do you agree that the current code in the scsi_debug driver follows the
ZBC standard and also that the above changes would break compatibility
with the ZBC standard?

Thanks,

Bart.

Re: [PATCH] scsi_debug: add implicit zones in max_open check

Posted by Andrew Bernal 2 months, 1 week ago

Hi Bart,
Yes, I've read the real standard now, and I see how it clearly defines
the OPEN ZONE function.
Thank you for your reply, I really appreciate the explanation. Sorry
for the patch, I'll be more careful in the future.

(my previous reply was sent to the email addresses, but didn't send to
the mailing lists because it wasn't plaintext)

Sincerely,
Andrew Bernal


On Tue, Jul 29, 2025 at 12:19 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 7/22/25 1:32 PM, Andrew Bernal wrote:
> > https://zonedstorage.io/docs/introduction/zoned-storage Open Zones limit
> > is defined as a "limit on the total number of zones that can simultaneously
> > be in an implicit open or explicit open state"
>
> That's not an official standard and hence should not be used to motivate
> this patch. Additionally, I don't see how a zone could be simultaneously
> in the implicit open and the explicit open state. According to the ZBC-2
> standard, these states are mutually exclusive.
>
> devip->max_open is reported to the initiator in VPD page B6
> as the MAXIMUM NUMBER OF OPEN SEQUENTIAL WRITE REQUIRED ZONES. From
> ZBC-2 section 4.5.3.4.2: "If the value in the MAXIMUM NUMBER OF OPEN
> SEQUENTIAL WRITE REQUIRED ZONES field (see 6.5.2) is non-zero and
> the number of zones with a Zone Condition of EXPLICITLY OPENED is equal
> to the value in the MAXIMUM NUMBER OF OPEN SEQUENTIAL WRITE REQUIRED
> ZONES field, then a command that writes to or attempts to open a
> sequential write required zone with a zone condition of EMPTY or CLOSED
> is terminated with CHECK CONDITION status with sense key set to DATA
> PROTECT and the additional sense code set to INSUFFICIENT ZONE RESOURCES
> (see 4.5.3.2.8)."
>
> > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> > index aef33d1e346a..0edb9a4698ca 100644
> > --- a/drivers/scsi/scsi_debug.c
> > +++ b/drivers/scsi/scsi_debug.c
> > @@ -3943,7 +3943,7 @@ static int check_zbc_access_params(struct scsi_cmnd *scp,
> >       /* Handle implicit open of closed and empty zones */
> >       if (zsp->z_cond == ZC1_EMPTY || zsp->z_cond == ZC4_CLOSED) {
> >               if (devip->max_open &&
> > -                 devip->nr_exp_open >= devip->max_open) {
> > +                 devip->nr_imp_open + devip->nr_exp_open >= devip->max_open) {
> >                       mk_sense_buffer(scp, DATA_PROTECT,
> >                                       INSUFF_RES_ASC,
> >                                       INSUFF_ZONE_ASCQ);
> > @@ -6101,7 +6101,7 @@ static int resp_open_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
> >       if (all) {
> >               /* Check if all closed zones can be open */
> >               if (devip->max_open &&
> > -                 devip->nr_exp_open + devip->nr_closed > devip->max_open) {
> > +                 devip->nr_imp_open + devip->nr_exp_open + devip->nr_closed > devip->max_open) {
> >                       mk_sense_buffer(scp, DATA_PROTECT, INSUFF_RES_ASC,
> >                                       INSUFF_ZONE_ASCQ);
> >                       res = check_condition_result;
> > @@ -6136,7 +6136,7 @@ static int resp_open_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
> >       if (zc == ZC3_EXPLICIT_OPEN || zc == ZC5_FULL)
> >               goto fini;
> >
> > -     if (devip->max_open && devip->nr_exp_open >= devip->max_open) {
> > +     if (devip->max_open && devip->nr_imp_open + devip->nr_exp_open >= devip->max_open) {
> >               mk_sense_buffer(scp, DATA_PROTECT, INSUFF_RES_ASC,
> >                               INSUFF_ZONE_ASCQ);
> >               res = check_condition_result;
>
> Do you agree that the current code in the scsi_debug driver follows the
> ZBC standard and also that the above changes would break compatibility
> with the ZBC standard?
>
> Thanks,
>
> Bart.