Skip to content

webadmin: added filter to display only not in use LUNs - #1006

Open
ekononov-dev wants to merge 2 commits into
oVirt:masterfrom
ekononov-dev:lun_filter_in_domain_management_window
Open

webadmin: added filter to display only not in use LUNs#1006
ekononov-dev wants to merge 2 commits into
oVirt:masterfrom
ekononov-dev:lun_filter_in_domain_management_window

Conversation

@ekononov-dev

Copy link
Copy Markdown
Contributor

Changes introduced with this PR

An existing problem:
When creating a new storage domain, when selecting a LUN, both unused LUNs and used LUNs are shown in the list. Since the list of LUNs can be very large, it is very difficult to search for unused LUNs that can be selected for a storage domain. This happens in the following cases:
- selecting a LUN when creating a new storage domain;
- selecting a LUN when editing an existing storage domain;
- selecting a LUN when creating RDM disks for a VM.

Problem solving:
The filter "Hide used LUNs" is added to the list of LUNs. When this filter is checked, the list shows only unused LUNs that can be added to the storage domain. This filter is checked by default.

Implementation features:
The implementation allows you to expand the LUNs filter, that is, to add additional filter fields.
All interfaces and classes listed below are located in the package org.ovirt.engine.ui.common.widget.ui common.storage.

The ModelFilter interface is the basic filter interface for any entities extending EntityModel. Two filters for LunModel implement the ModelFilter interface: LunFilter (LUN filter itself) and SanTargetFilter (SanTarget filter by the LUN property, which are located inside SanTarget). These filters also extend the basic LUN filter class LunFilterBase, which defines the fields of the LUN filter.

For adding additional LUN filter fields:
- add new filter fields to LunFilterBase;
- extend the LunFilterBase#needFilter() function by checking for new filter fields;
- extend the ModelFilter#filter() function with new filter fields;
- add new filter fields to the UI.

Are you the owner of the code you are sending in, or do you have permission of the owner?

Yes

@ekononov-dev
ekononov-dev requested a review from sgratch as a code owner April 4, 2025 12:50
@dupondje

dupondje commented Apr 4, 2025

Copy link
Copy Markdown
Member

I like the idea. Will try to review asap! :)

@JasperB-TeamBlue JasperB-TeamBlue left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When filtering and no items are found the engine mostly displays "no items to display", this functionality is still missing for this filtering patch.

@ekononov-dev

Copy link
Copy Markdown
Contributor Author

When filtering and no items are found the engine mostly displays "no items to display", this functionality is still missing for this filtering patch.

In this case, the Tree widget is used to display the list of LUNs/targets, which does not have the built-in function to display "no items to display" . Also, as part of this PR, an additional "Hide used LUNs" filter was only added, and the widget's behavior when there is no data to display remained as it was before.

In my opinion, adding the ability to display "no items to display" should be implemented as part of some another PR, OK?

@ekononov-dev
ekononov-dev force-pushed the lun_filter_in_domain_management_window branch from 5a5da05 to b9c3bb5 Compare May 4, 2025 19:18
An existing problem:
When creating a new storage domain, when selecting a LUN, both unused LUNs and used LUNs are shown in the list. Since the list of LUNs can be very large, it is very difficult to search for unused LUNs that can be selected for a storage domain.
This happens in the following cases:
	- selecting a LUN when creating a new storage domain;
	- selecting a LUN when editing an existing storage domain;
	- selecting a LUN when creating RDM disks for a VM.

Problem solving:
The filter "Hide used LUNs" is added to the list of LUNs. When this filter is checked, the list shows only unused LUNs that can be added to the storage domain. This filter is checked by default.

Signed-off-by: Evgeniy Kononov <ekononov1919@gmail.com>
@ekononov-dev
ekononov-dev force-pushed the lun_filter_in_domain_management_window branch from b9c3bb5 to 60f0784 Compare May 5, 2025 14:19

@JasperB-TeamBlue JasperB-TeamBlue left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested changes were added and functionality works properly. LGTM.

@ekononov-dev

Copy link
Copy Markdown
Contributor Author

Requested changes were added and functionality works properly. LGTM.

Can I close resolved conversations? Or you will do this by yourself?

@JasperB-TeamBlue

Copy link
Copy Markdown
Collaborator

Requested changes were added and functionality works properly. LGTM.

Can I close resolved conversations? Or you will do this by yourself?

Go ahead, I do not seem to have the option to close these conversations.

@ekononov-dev

Copy link
Copy Markdown
Contributor Author

Requested changes were added and functionality works properly. LGTM.

Can I close resolved conversations? Or you will do this by yourself?

Go ahead, I do not seem to have the option to close these conversations.

I've closed it

@ekononov-dev

Copy link
Copy Markdown
Contributor Author

I like the idea. Will try to review asap! :)

Hello!
Are we waiting for other reviewers?

@JasperB-TeamBlue

Copy link
Copy Markdown
Collaborator

/ost

@github-actions

Copy link
Copy Markdown

⏳ Running ost suite 'basic-suite-master' on distro 'el9stream'.

Follow the progress here.

@github-actions

Copy link
Copy Markdown

😭💔 ost suite 'basic-suite-master' on distro 'el9stream' failed. (details)

Signed-off-by: ekononov-dev <ekononov1919@gmail.com>
@ekononov-dev

Copy link
Copy Markdown
Contributor Author

Conflicts have been resolved.

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

Successfully merging this pull request may close these issues.

3 participants