Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doc: add doc of new fcp_template and attach/detach APIs #772

Open
dongyanyang opened this issue Nov 23, 2023 · 9 comments
Open

Doc: add doc of new fcp_template and attach/detach APIs #772

dongyanyang opened this issue Nov 23, 2023 · 9 comments
Assignees

Comments

@dongyanyang
Copy link
Contributor

The doc of CRUD of fcp_template and the attach_volume/detach_volume APIs are missing, need to add them.

@dongyanyang dongyanyang self-assigned this Nov 23, 2023
@Bischoff
Copy link
Contributor

Bischoff commented Dec 30, 2023

@dongyanyang thank you for your efforts for documenting.

However, I stumbled on various problems with the documentation of "Attach volume" and subsequent sections.

I report these problems below (one problem per comment).

@Bischoff
Copy link
Contributor

Bischoff commented Dec 31, 2023

1/7 - In the doc for "Attach volume" and "Detach volume", in the parameters list, one reads:

zvm_fcp  body  list FCP devices list.

however, in the example:

        "zvm_fcp": "1fc5",

This is a string, not a list. I think the example should be instead:

        "zvm_fcp": [ "1fc5" ],

@Bischoff
Copy link
Contributor

Bischoff commented Dec 31, 2023

2/7 - When I try to run the example for "Attach volume", I get:

HTTP status: 400, body: {"overallRC": 400, "rc": 400, "rs": 400, "modID": 120, "output": "", "errmsg":
 "ValidationError: Validation error: Invalid input for field/attribute target_wwpn. Value:
 0x50050763050b073d. '0x50050763050b073d' is not of type 'array'"}

That means that both doc and example are wrong. Doc should be:

target_wwpn  body  list  World Wide Port Numbers, must start with 0x, for example 0x50050763050b073d

(with list instead of string, and plural), and example should be:

       "target_wwpn": [ "0x50050763050b073d" ],

@Bischoff
Copy link
Contributor

Bischoff commented Dec 31, 2023

3/7 - Also, when running the example for "Attach volume", I get:

HTTP status: 500, body: {"overallRC": 500, "modID": 100, "rc": 500, "rs": 1, "errmsg":
 "Unexpected internal error in ZVM SDK, error: (127.0.0.1:49340) SDK server got unexpected exception:
 KeyError('fcp_template_id')", "output": ""}

The problem goes away when adding a string parameter "fcp_template_id": to the JSON, but this parameter is neither documented nor in the example.

According to source code, fcp_template_id is also a parameter of "Set FCP usage", but I don't see it documented there either.

@Bischoff
Copy link
Contributor

Bischoff commented Dec 31, 2023

4/7 - The following sections:

7.5.8. Refresh Volume Bootmap Info
PUT /volumes/volume_refresh_bootmap

7.5.9. Get Volume Connector
GET /volumes/conn/{userid}

7.5.10. Get FCP Usage
GET /volumes/fcp/{fcp_id}

7.5.11. Set FCP Usage
PUT /volumes/fcp/{fcp_id}

should not be under section 7.5 Guest(s), but under a new section 7.x Volume(s), because the REST path starts with /volumes instead of /guests.

@Bischoff
Copy link
Contributor

Bischoff commented Dec 31, 2023

5/7 - In section 7.5.8. Refresh Volume Bootmap Info, one reads:

wwpn   body   string  World Wide Port Name IDs.

I think it should be list instead of string, because the example states:

            "wwpn": ['5005076802100c1a', '5005076802200c1b'],

@Bischoff
Copy link
Contributor

Bischoff commented Dec 31, 2023

6/7 - Section https://cloudlib4zvm.readthedocs.io/en/latest/restapi.html#get-volume-connector mentions a parameter named reserve. Shouldn't it also mention the following optional parameters?

  fcp_template_id
  storage_provider
  pchid_info

@Bischoff
Copy link
Contributor

Bischoff commented Dec 31, 2023

7/7 - In "Set FCP usage", one reads:

userid  path  string  Guest userid

According to the source code, it seems that userid should be in body rather than in path.

@jackydalong
Copy link
Contributor

Hi @Bischoff .
Sorry for the out-of-dated doc. We are discussing on updating it soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants