Re: [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores

[Dietmar Maurer <dietmar@proxmox.com>]

more comments inline:

> +pub fn do_mount_device(
> +    _lock: Option<BackupLockGuard>,
> +    mut config: SectionConfigData,
> +    mut datastore: DataStoreConfig,
> +    worker: Option<&dyn WorkerTaskContext>,
> +) -> Result<(), Error> {
> +    if let Some(uuid) = datastore.backing_device.as_ref() {
> +        if pbs_datastore::check_if_available(&datastore).is_ok() {
> +            return Err(format_err!("device '{}' is already mounted", &uuid));
> +        }
> +        let mount_point_path = std::path::Path::new(&datastore.path);
> +        if let Some(worker) = worker {
> +            task_log!(worker, "mounting '{}' to '{}'", uuid, datastore.path);
> +        }
> +        crate::tools::disks::mount_by_uuid(uuid, mount_point_path)?;
> +
> +        datastore.set_maintenance_mode(None);

You change the maintenance mode, but do not notify the proxy?

> +        config.set_data(&datastore.name, "datastore", &datastore)?;
> +        pbs_config::datastore::save_config(&config)?;
> +
> +        Ok(())
> +    } else {
> +        Err(format_err!(
> +            "Datastore '{}' can't be mounted because it is not removable.",
> +            &datastore.name
> +        ))
> +    }
> +}
> +
> +#[api(
> +    protected: true,
> +    input: {
> +        properties: {
> +            store: {
> +                schema: DATASTORE_SCHEMA,
> +            },
> +        }
> +    },
> +    returns: {
> +        schema: UPID_SCHEMA,
> +    },
> +    access: {
> +        permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false),
> +    },
> +)]
> +/// Mount removable datastore.
> +pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> +    let (section_config, _digest) = pbs_config::datastore::config()?;
> +    let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> +
> +    if datastore.backing_device.is_none() {
> +        bail!("datastore '{}' is not removable", &store);
> +    }
> +
> +    let lock = pbs_config::datastore::lock_config()?;
> +    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> +
> +    let upid = WorkerTask::new_thread(
> +        "mount-device",
> +        Some(store),
> +        auth_id.to_string(),
> +        to_stdout,
> +        move |worker| do_mount_device(Some(lock), section_config, datastore, Some(&worker)),
> +    )?;
> +
> +    Ok(json!(upid))
> +}
> +
> +fn do_unmount_device(
> +    force: bool,
> +    datastore: DataStoreConfig,
> +    worker: Option<&dyn WorkerTaskContext>,
> +) -> Result<(), Error> {
> +    if force {
> +        let _ = crate::tools::disks::unmount_by_mountpoint(&datastore.path);
> +        return Ok(());
> +    }
> +
> +    let mut active_operations = task_tracking::get_active_operations(&datastore.name)?;
> +    while active_operations.read + active_operations.write > 0 {
> +        if let Some(worker) = worker {
> +            if worker.abort_requested() {
> +                bail!("aborted, due to user request");
> +            }
> +            task_log!(
> +                worker,
> +                "can't unmount yet, still {} read and {} write operations active",
> +                active_operations.read,
> +                active_operations.write
> +            );
> +        }
> +
> +        std::thread::sleep(std::time::Duration::new(5, 0));
> +        active_operations = task_tracking::get_active_operations(&datastore.name)?;
> +    }
> +    crate::tools::disks::unmount_by_mountpoint(&datastore.path)?;
> +
> +    Ok(())
> +}
> +
> +#[api(
> +    protected: true,
> +    input: {
> +        properties: {
> +            store: { schema: DATASTORE_SCHEMA },
> +            force: {
> +                type: Boolean,
> +                description: "Force unmount even if there are active operations.",
> +                optional: true,
> +                default: false,
> +            },
> +        },
> +    },
> +    returns: {
> +        schema: UPID_SCHEMA,
> +    },
> +    access: {
> +        permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
> +    }
> +)]
> +/// Unmount a removable device that is associated with the datastore
> +pub async fn unmount(
> +    store: String,
> +    force: bool,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Value, Error> {
> +    let _lock = pbs_config::datastore::lock_config()?;
> +    let (mut section_config, _digest) = pbs_config::datastore::config()?;
> +    let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> +
> +    if datastore.backing_device.is_none() {
> +        bail!("datastore '{}' is not removable", &store);
> +    }
> +
> +    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> +
> +    datastore.set_maintenance_mode(Some(MaintenanceMode::new(MaintenanceType::Unplugged, None)));
> +    section_config.set_data(&datastore.name, "datastore", &datastore)?;
> +    pbs_config::datastore::save_config(&section_config)?;
> +    drop(_lock);
> +
> +    if let Ok(proxy_pid) = proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN)
> +    {
> +        let sock = proxmox_rest_server::ctrl_sock_from_pid(proxy_pid);
> +        let _ = proxmox_rest_server::send_raw_command(
> +            sock,
> +            &format!(
> +                "{{\"command\":\"update-datastore-cache\",\"args\":\"{}\"}}\n",
> +                &store
> +            ),
> +        )
> +        .await;
> +    }

I found the same code in strc/api2/config/datastore.rs. I guess its is worth to factor this out.