fix(storage): reprobe and recalculate proposal

service/lib/agama/dbus/storage/manager.rb

@@ -88,7 +88,13 @@ def issues
         private_constant :STORAGE_INTERFACE
 
         def probe
-          busy_while { backend.probe }
+          busy_while do
+            # Clean trees in advance to avoid having old objects exported in D-Bus.
+            system_devices_tree.clean
+            staging_devices_tree.clean
+
+            backend.probe
+          end
         end
 
         # Applies the given serialized config according to the JSON schema.

service/package/rubygem-agama-yast.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Fri Jan 10 15:44:30 UTC 2025 - José Iván López González <jlopez@suse.com>
+
+- Objects from the D-Bus trees representing the storage devices are
+  removed before performing the probing. It prevents a segmentation
+  fault by accessing to old objects (gh#agama-project/agama#1884).
+
 -------------------------------------------------------------------
 Thu Jan  9 12:21:40 UTC 2025 - Knut Anderssen <kanderssen@suse.com>
 

web/package/agama-web-ui.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Fri Jan 10 15:56:35 UTC 2025 - José Iván López González <jlopez@suse.com>
+
+- Add storage reprobing and recalculate proposal when going back to
+  either the proposal page or the devices selector if the system
+  is deprecated (gh#agama-project/agama#1884).
+
 -------------------------------------------------------------------
 Fri Jan 10 13:46:42 UTC 2025 - David Diaz <dgonzalez@suse.com>
 

web/src/api/storage.ts

@@ -32,8 +32,6 @@ import { config } from "~/api/storage/types";
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 const probe = (): Promise<any> => post("/api/storage/probe");
 
-export { probe };
-
 const fetchConfig = (): Promise<config.Config | undefined> =>
   get("/api/storage/config").then((config) => config.storage);
 
@@ -57,8 +55,8 @@ const findStorageJob = (id: string): Promise<Job | undefined> =>
  */
 const refresh = async (): Promise<void> => {
   const settings = await fetchSettings();
-  await probe();
-  await calculate(settings);
+  await probe().catch(console.log);
+  await calculate(settings).catch(console.log);
 };
 
-export { fetchConfig, fetchStorageJobs, findStorageJob, refresh };
+export { probe, fetchConfig, fetchStorageJobs, findStorageJob, refresh };

web/src/components/storage/DeviceSelection.tsx

@@ -27,11 +27,17 @@ import { Page } from "~/components/core";
 import { DeviceSelectorTable } from "~/components/storage";
 import DevicesTechMenu from "./DevicesTechMenu";
 import { ProposalTarget, StorageDevice } from "~/types/storage";
-import { useAvailableDevices, useProposalMutation, useProposalResult } from "~/queries/storage";
+import {
+  useAvailableDevices,
+  useProposalMutation,
+  useProposalResult,
+  useRefresh,
+} from "~/queries/storage";
 import { deviceChildren } from "~/components/storage/utils";
 import { compact } from "~/utils";
 import a11y from "@patternfly/react-styles/css/utilities/Accessibility/accessibility";
 import { _ } from "~/i18n";
+import { Loading } from "~/components/layout";
 
 const SELECT_DISK_ID = "select-disk";
 const CREATE_LVM_ID = "create-lvm";
@@ -53,11 +59,17 @@ export default function DeviceSelection() {
   const availableDevices = useAvailableDevices();
   const updateProposal = useProposalMutation();
   const navigate = useNavigate();
+  const [isLoading, setIsLoading] = useState<boolean>(false);
   const [state, setState] = useState<DeviceSelectionState>({});
 
   const isTargetDisk = state.target === ProposalTarget.DISK;
   const isTargetNewLvmVg = state.target === ProposalTarget.NEW_LVM_VG;
 
+  useRefresh({
+    onStart: () => setIsLoading(true),
+    onFinish: () => setIsLoading(false),
+  });
+
   useEffect(() => {
     if (state.target !== undefined) return;
 
@@ -118,6 +130,8 @@ physical volumes will be created on demand as new partitions at the selected \
 devices.",
   ).split(/[[\]]/);
 
+  if (isLoading) return <Loading />;
+
   return (
     <Page>
       <Page.Header>

web/src/components/storage/ISCSIPage.tsx

@@ -24,6 +24,7 @@ import { Grid, GridItem } from "@patternfly/react-core";
 import React from "react";
 import { Page } from "~/components/core";
 import { InitiatorSection, TargetsSection } from "~/components/storage/iscsi";
+import { STORAGE as PATHS } from "~/routes/paths";
 import { _ } from "~/i18n";
 
 export default function ISCSIPage() {
@@ -42,6 +43,11 @@ export default function ISCSIPage() {
           </GridItem>
         </Grid>
       </Page.Content>
+      <Page.Actions>
+        <Page.Action variant="secondary" navigateTo={PATHS.targetDevice}>
+          {_("Back to device selection")}
+        </Page.Action>
+      </Page.Actions>
     </Page>
   );
 }

web/src/components/storage/ProposalPage.tsx

@@ -20,7 +20,7 @@
  * find current contact information at www.suse.com.
  */
 
-import React, { useRef } from "react";
+import React, { useRef, useState } from "react";
 import { Grid, GridItem, Stack } from "@patternfly/react-core";
 import { Page, Drawer, EmptyState } from "~/components/core/";
 import ProposalTransactionalInfo from "./ProposalTransactionalInfo";
@@ -35,16 +35,14 @@ import { useIssues } from "~/queries/issues";
 import { IssueSeverity } from "~/types/issues";
 import {
   useAvailableDevices,
-  useDeprecated,
   useDevices,
   useProductParams,
   useProposalMutation,
   useProposalResult,
   useVolumeDevices,
   useVolumeTemplates,
+  useRefresh,
 } from "~/queries/storage";
-import { useQueryClient } from "@tanstack/react-query";
-import { refresh } from "~/api/storage";
 
 const StorageWarning = () => (
   <Page>
@@ -96,16 +94,12 @@ export default function ProposalPage() {
   const { encryptionMethods } = useProductParams({ suspense: true });
   const proposal = useProposalResult();
   const updateProposal = useProposalMutation();
-  const deprecated = useDeprecated();
-  const queryClient = useQueryClient();
+  const [isLoading, setIsLoading] = useState<boolean>(false);
 
-  React.useEffect(() => {
-    if (deprecated) {
-      refresh().then(() => {
-        queryClient.invalidateQueries({ queryKey: ["storage"] });
-      });
-    }
-  }, [deprecated, queryClient]);
+  useRefresh({
+    onStart: () => setIsLoading(true),
+    onFinish: () => setIsLoading(false),
+  });
 
   const errors = useIssues("storage")
     .filter((s) => s.severity === IssueSeverity.Error)
@@ -141,7 +135,7 @@ export default function ProposalPage() {
               volumeTemplates={volumeTemplates}
               settings={settings}
               onChange={changeSettings}
-              isLoading={false}
+              isLoading={isLoading}
             />
           </GridItem>
           <GridItem sm={12} xl={6}>
@@ -162,14 +156,14 @@ export default function ProposalPage() {
                   // @ts-expect-error: we do not know how to specify the type of
                   // drawerRef properly and TS does not find the "open" property
                   onActionsClick={drawerRef.current?.open}
-                  isLoading={false}
+                  isLoading={isLoading}
                 />
                 <ProposalResultSection
                   system={systemDevices}
                   staging={stagingDevices}
                   actions={actions}
                   errors={errors}
-                  isLoading={false}
+                  isLoading={isLoading}
                 />
               </Stack>
             </Drawer>

web/src/queries/storage.ts

@@ -29,7 +29,7 @@ import {
 } from "@tanstack/react-query";
 import React from "react";
 import { fetchDevices, fetchDevicesDirty } from "~/api/storage/devices";
-import { fetchConfig } from "~/api/storage";
+import { fetchConfig, refresh } from "~/api/storage";
 import {
   calculate,
   fetchActions,
@@ -361,6 +361,32 @@ const useDeprecatedChanges = () => {
   });
 };
 
+type RefreshHandler = {
+  onStart?: () => void;
+  onFinish?: () => void;
+};
+
+/**
+ * Hook that reprobes the devices and recalculates the proposal using the current settings.
+ */
+const useRefresh = (handler?: RefreshHandler) => {
+  const queryClient = useQueryClient();
+  const deprecated = useDeprecated();
+
+  handler ||= {};
+  handler.onStart ||= () => undefined;
+  handler.onFinish ||= () => undefined;
+
+  React.useEffect(() => {
+    if (!deprecated) return;
+
+    handler.onStart();
+    refresh()
+      .then(() => queryClient.invalidateQueries({ queryKey: ["storage"] }))
+      .then(() => handler.onFinish());
+  }, [handler, deprecated, queryClient]);
+};
+
 export {
   useConfig,
   useDevices,
@@ -372,4 +398,5 @@ export {
   useProposalMutation,
   useDeprecated,
   useDeprecatedChanges,
+  useRefresh,
 };