openmainframeproject / feilong

Feilong is a open source z/VM cloud connector project under the Open Mainframe Project umbrella that will accelerate the z/VM adoption, extending its ecosystem and its user experience. It provides a set of APIs to operate z/VM including guest, image, network, volume etc.
https://www.openmainframeproject.org/projects/feilong
Apache License 2.0
35 stars 70 forks source link

Add API call to get minidisks list #804

Closed Bischoff closed 5 months ago

Bischoff commented 7 months ago

This PR adds missing GET operation for the list of minidisks of a guest:

GET /guests/{userid}/disks

This PR solves issue #796.

It contains changes to:

Bischoff commented 5 months ago

Another quesion, how to deal with temporary linked MDISK (not owned by this guest vm). Whether such linked MDISK info should be included in the output of the new API.

We could have a flag wether to include them or not. Or just always include them. But the question is whether it would be cleaner to have a different call for them...

Admitting we decide to do it, how could I access that information on the SMAPI side?

bjhuangr commented 5 months ago

The same API Image_Disk_Query is able to query MDISKs that linked from other guest.

I'm fine to add a flag for include/exclude the disks not owned by the target guest.

Bischoff commented 5 months ago

The same API Image_Disk_Query is able to query MDISKs that linked from other guest.

Done, it works fine too.

I'm fine to add a flag for include/exclude the disks not owned by the target guest.

I don't see the way to distinguish between local or linked, neither in input nor in output.

For the output, here is what the API call returns:

 DASD VDEV: 0100
   RDEV: 6018
   Access type: R/W
   Device type: 3390
   Device size: 17477
   Device units: Cylinders
   Device volume label: VM6018

 DASD VDEV: 0190
   RDEV: 6000
   Access type: R/O
   Device type: 3390
   Device size: 214
   Device units: Cylinders
   Device volume label: M01RES

 DASD VDEV: 019D
   RDEV: 6000
   Access type: R/O
   Device type: 3390
   Device size: 292
   Device units: Cylinders
   Device volume label: M01RES

 DASD VDEV: 019E
   RDEV: 6000
   Access type: R/O
   Device type: 3390
   Device size: 500
   Device units: Cylinders
   Device volume label: M01RES

 DASD VDEV: 0592
   RDEV: 6000
   Access type: R/O
   Device type: 3390
   Device size: 240
   Device units: Cylinders
   Device volume label: M01RES

I could reintroduce the call to Image_Query_DM that you made me remove to get a list of directly connected vdevs, but that would lead to complicated code where I would need to match the output of the two API calls...

bjhuangr commented 5 months ago

Is there a customer requirement that rely on this new API? If this is not the case, adding this API just because we missed this part. Then I suggest we can return all guest vm awared disks information at first.

Bischoff commented 5 months ago

Is there a customer requirement that rely on this new API?

Yes, for the terraform provider. Without this API call, the provider cannot inspect the current situation to check if some disk has been added outside of the provider.

See code here https://github.com/Bischoff/terraform-provider-feilong/blob/e38c5ee7d461b1673c90148f7c6836c30f560d8e/provider/feilong_guest.go#L289

If this is not the case, adding this API just because we missed this part.

There's a real need - and yes, it was also missed :smile_cat: .

Then I suggest we can return all guest vm awared disks information at first.

I am not sure to understand. Do you want to return first the disks allocated to the guest, and after that the linked ones?

If that is your request, sure, it's a good idea - but same question: how?

In my tests, SMAPI already returns the disk created with the guest first. But it might only be a coincidence, because the allocated disk has address 0100 while all the others have addresses above (0190 etc).

bjhuangr commented 5 months ago

I meant, we can start from easy approach - return all disks info.

Bischoff commented 5 months ago

I meant, we can start from easy approach - return all disks info.

Of course. That's the current state of this PR.

What I can do later on is to add a boolean field to the returned structures: directly connected yes/no. That way, we would keep an ascending compatibility.