search

opiproject / opi-api

Open Programmable Infrastructure API and Behavioral Model
Apache License 2.0
31 stars 40 forks source link

feat(storage): set annotations for backend #330

Closed artek-koltun closed 1 year ago

artek-koltun commented 1 year ago

See https://github.com/opiproject/opi-api/issues/198 and AIP-203

First, let's try to discuss where we are going to keep functionality to connect to local Nvme PCIe drives for JBOF case, since it would affect how to annotate the fields properly. I see an empty separate backend_nvme_pcie.proto, but also I see NVME_TRANSPORT_PCIE enum value in NvmeTransportType in backend_nvme_tcp.proto file, which might imply reuse of the service for PCIe.

Some assumptions/considerations in the first patch:

  1. Assumed we can reuse NvmeRemoteControllerService for PCIe
  2. FabricsPath message was created, which is part of NvmePath. It allows to annotate some fields as REQUIRED for Nvme over fabrics transport types and have more consistent API. Otherwise, we need to keep them OPTIONAL, since we can use the same API to connect to local PCIe.
  3. TcpController was created to be consistent with previous item and isolate TCP related optional parameters. This message is a part of NvmeRemoteController.
artek-koltun commented 1 year ago

the annotations looks ok, but creating new message TcpController should be discussed and in a separate PR

ok, I can remove TcpController for a while. However, I want we all to agree on where we want to keep pcie related part: in the existing NvmeRemoteControllerService service or in a dedicated one within backend_nvme_pcie.proto @jainvipin @sburla-marvell @GottliebNoam?

glimchb commented 1 year ago

@artek-koltun yes, please remove TcpController for now and we will add have this discussion