Re: [PATCH 03/17] nvme: Enforce extended LBA format for fabrics metadata

[Christoph Hellwig <hch@xxxxxx>]

On Fri, Mar 27, 2020 at 08:15:31PM +0300, Max Gurtovoy wrote:
>  		/*
> +		 * For Fabrics, only metadata as part of extended data LBA is
> +		 * supported. Fail in case of a spec violation.
> +		 */
> +		if (ns->ctrl->ops->flags & NVME_F_FABRICS) {
> +			if (WARN_ON_ONCE(!(ns->features & NVME_NS_EXT_LBAS)))
> +				return -EINVAL;
> +		}
> +
> +		/*
>  		 * For PCI, Extended logical block will be generated by the
>  		 * controller.
>  		 */
>  		if (ns->ctrl->ops->flags & NVME_F_METADATA_SUPPORTED) {
> +			if (ns->ctrl->ops->flags & NVME_F_FABRICS ||
> +			    !(ns->features & NVME_NS_EXT_LBAS))
>  				ns->features |= NVME_NS_MD_HOST_SUPPORTED;

This looks a little confusing.  I think it might make more sense


	struct nvme_ctrl *ctrl = ns->ctrl;

	...

	if (ns->ms) {
  		/*
		 * For PCIe only the separate metadata pointer is supported,
		 * as the block layer supplies metadata in a separate
		 * bio_vec chain.  For Fabrics, only metadata as part of
		 * extended data LBA is supported on the wire per the Fabrics
		 * specification, but the HBA/HCA will do the remapping from
		 * the separate metadata buffers for us.
		 */
		if (id->flbas & NVME_NS_FLBAS_META_EXT) {
			ns->features |= NVME_NS_EXT_LBAS;
			if ((ctrl->ops->flags & NVME_F_FABRICS) &&
			    (ctrl->ops->flags & NVME_F_METADATA_SUPPORTED))
 				ns->features |= NVME_NS_METADATA_SUPPORTED;
		} else {
			if (WARN_ON_ONCE(ctrl->ops->flags & NVME_F_FABRICS))
				return -EINVAL;
			if (ctrl->ops->flags & NVME_F_METADATA_SUPPORTED)
 				ns->features |= NVME_NS_METADATA_SUPPORTED;
				
		}