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

Extend storage D-Bus API #1071

Merged

Conversation

joseivanlopez
Copy link
Contributor

@joseivanlopez joseivanlopez commented Mar 5, 2024

Problem

The current result of the storage proposal is presented as a list of actions, which could make a bit complex to understand the overall picture of the resulting devices.

That list of actions is going to be replaced by a more visual representation of the devices. To make this possible, more information about the devices has to be exported on D-Bus.

Solution

Add the following information to D-Bus:

  • The list of actions includes the SID of the affected device.
  • The partition table exports the unused slots.
  • The LVM devices (volume groups, physical volumes and logical volumes) are exported.
  • The block devices includes their start block and also indicates whether the device is encrypted.
  • The staging devices are exported.

NOTE: This PR goes to a feature branch instead of master.

Testing

  • Added unit tests
  • Tested manually

- agama.gemspec was renamed, see
  agama-project#1056.
- The file name was not updated in the rubocop config.
@joseivanlopez joseivanlopez marked this pull request as ready for review March 6, 2024 15:20
attr_reader :storage_device
# sid of the Y2Storage device.
#
# @note A Y2Storage device is a wrapper over a libstorage-ng object. If the source
Copy link
Contributor

@ancorgs ancorgs Mar 6, 2024

Choose a reason for hiding this comment

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

Isn't this explanation a bit too technical and a bit too close to the Y2Storage world? In fact, I would say that using the sid simply makes sense here and does not need that explanation about memory.

Just something saying this is the internal storage identifier and clarifying it's unique per device should be enough.

Copy link
Contributor Author

@joseivanlopez joseivanlopez Mar 6, 2024

Choose a reason for hiding this comment

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

Yes, it is. But I would prefer to keep it as a warning. I see myself trying to use storage_device again for something, which is quite unsafe.

Copy link
Contributor

@ancorgs ancorgs left a comment

Choose a reason for hiding this comment

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

LGTM. I expected this to be more complicated, nice to see we actually have a nice infrastructure in place.

@joseivanlopez joseivanlopez merged commit e35d470 into agama-project:storage-ui Mar 6, 2024
3 checks passed
This was referenced Mar 7, 2024
joseivanlopez added a commit that referenced this pull request Mar 8, 2024
## Problem

More information is required to properly represent the storage proposal
result:

* Device description
* Filesystem label

This is a follow-up of #1071.

## Solution

Add new D-Bus interfaces `Device` , `Partition`, `LVM.LogicalVolume` and
adapt `Filesystem` interface.

This changes require yast/yast-storage-ng#1373.

NOTE: This PR goes to a feature branch instead of master.

## Testing

* Added unit tests
* Tested manually
joseivanlopez added a commit that referenced this pull request Mar 11, 2024
## Problem

The storage D-Bus API was extended to provide all the information
required for a better representation of the proposal result, see
#1071 and
#1082. The JS storage client has
to be adapted.

## Solution

Adapt the storage client to the changes in the D-Bus API and adapt
`ProposalPage` component to read the information about the devices if
needed.

NOTE: This PR goes to a feature branch instead of master.

## Testing

* Added new unit tests
* Tested manually
dgdavid added a commit that referenced this pull request Mar 19, 2024
As part of the Storage UI changes described at
https://github.com/openSUSE/agama/blob/master/doc/storage_ui.md
document, this PR aims to merge into master the changes already
available in the `storage_ui` feature branch, which has been already
tested and validated. Namely,

* #1071

   Added the following information to D-Bus:

    * The list of actions includes the SID of the affected device.
    * The partition table exports the unused slots.
* The LVM devices (volume groups, physical volumes and logical volumes)
are exported.
* The block devices includes their start block and also indicates
whether the device is encrypted.
    * The staging devices are exported.
    
* #1079 

Adapt the storage client to the changes in the D-Bus API and adapt
`ProposalPage` component to read the information about the devices if
needed.


* #1082

Added new D-Bus interfaces `Device` , `Partition`, `LVM.LogicalVolume`
and adapt `Filesystem` interface.

  It requires yast/yast-storage-ng#1373.

* #1088

Replaced the `Planned Actions` section in the storage proposal for a
`Result` one which presents how the storage would look after
installation instead of just the list of actions.

* #1090

  Moved the space policy configuration to a popup.


* #1098

Replace a `Resize` by `Before` label as it was suggested during the
presentation of the UI in a review meeting.
@imobachgs imobachgs mentioned this pull request May 17, 2024
imobachgs added a commit that referenced this pull request May 17, 2024
Prepare for releasing Agama 8. It includes the following pull requests:

* #884
* #886
* #914
* #918
* #956
* #957
* #958
* #959
* #960
* #961
* #962
* #963
* #964
* #965
* #966
* #969
* #970
* #976
* #977
* #978
* #979
* #980
* #981
* #983
* #984
* #985
* #986
* #988
* #991
* #992
* #995
* #996
* #997
* #999
* #1003
* #1004
* #1006
* #1007
* #1008
* #1009
* #1010
* #1011
* #1012
* #1014
* #1015
* #1016
* #1017
* #1020
* #1022
* #1023
* #1024
* #1025
* #1027
* #1028
* #1029
* #1030
* #1031
* #1032
* #1033
* #1034
* #1035
* #1036
* #1038
* #1039
* #1041
* #1042
* #1043
* #1045
* #1046
* #1047
* #1048
* #1052
* #1054
* #1056
* #1057
* #1060
* #1061
* #1062
* #1063
* #1064
* #1066
* #1067
* #1068
* #1069
* #1071
* #1072
* #1073
* #1074
* #1075
* #1079
* #1080
* #1081
* #1082
* #1085
* #1086
* #1087
* #1088
* #1089
* #1090
* #1091
* #1092
* #1093
* #1094
* #1095
* #1096
* #1097
* #1098
* #1099
* #1100
* #1102
* #1103
* #1104
* #1105
* #1106
* #1109
* #1110
* #1111
* #1112
* #1114
* #1116
* #1117
* #1118
* #1119
* #1120
* #1121
* #1122
* #1123
* #1125
* #1126
* #1127
* #1128
* #1129
* #1130
* #1131
* #1132
* #1133
* #1134
* #1135
* #1136
* #1138
* #1139
* #1140
* #1141
* #1142
* #1143
* #1144
* #1145
* #1146
* #1147
* #1148
* #1149
* #1151
* #1152
* #1153
* #1154
* #1155
* #1156
* #1157
* #1158
* #1160
* #1161
* #1162
* #1163
* #1164
* #1165
* #1166
* #1167
* #1168
* #1169
* #1170
* #1171
* #1172
* #1173
* #1174
* #1175
* #1177
* #1178
* #1180
* #1181
* #1182
* #1183
* #1184
* #1185
* #1187
* #1188
* #1189
* #1190
* #1191
* #1192
* #1193
* #1194
* #1195
* #1196
* #1198
* #1199
* #1200
* #1201
* #1203
* #1204
* #1205
* #1206
* #1207
* #1208
* #1209
* #1210
* #1211
* #1212
* #1213
* #1214
* #1215
* #1216
* #1217
* #1219
* #1220
* #1221
* #1222
* #1223
* #1224
* #1225
* #1226
* #1227
* #1229
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.

2 participants