all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup v3 0/2] removal of mount-units/directories
@ 2020-08-13 10:58 Hannes Laimer
  2020-08-13 10:58 ` [pbs-devel] [PATCH proxmox-backup v3 1/2] api2/node/../disks/directory: added DELETE endpoint for removal of mount-units Hannes Laimer
  2020-08-13 10:58 ` [pbs-devel] [PATCH proxmox-backup v3 2/2] ui/cli: added support for the " Hannes Laimer
  0 siblings, 2 replies; 6+ messages in thread
From: Hannes Laimer @ 2020-08-13 10:58 UTC (permalink / raw)
  To: pbs-devel

Added support for the removal of mount-units from the WebUI and Cli

Hannes Laimer (2):
  api2/node/../disks/directory: added DELETE endpoint for removal of
    mount-units
  ui/cli: added support for the removal of mount-units

 src/api2/node/disks/directory.rs       |  72 ++++++++-
 src/bin/proxmox_backup_manager/disk.rs |  29 +++-
 src/tools/systemd.rs                   |  11 ++
 www/DirectoryList.js                   | 150 ++++++++++--------
 www/Makefile                           |   1 +
 www/window/SafeRemove.js               | 209 +++++++++++++++++++++++++
 6 files changed, 406 insertions(+), 66 deletions(-)
 create mode 100644 www/window/SafeRemove.js

-- 
2.20.1





^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v3 1/2] api2/node/../disks/directory: added DELETE endpoint for removal of mount-units
  2020-08-13 10:58 [pbs-devel] [PATCH proxmox-backup v3 0/2] removal of mount-units/directories Hannes Laimer
@ 2020-08-13 10:58 ` Hannes Laimer
  2020-08-14  5:38   ` [pbs-devel] applied: " Dietmar Maurer
  2020-08-13 10:58 ` [pbs-devel] [PATCH proxmox-backup v3 2/2] ui/cli: added support for the " Hannes Laimer
  1 sibling, 1 reply; 6+ messages in thread
From: Hannes Laimer @ 2020-08-13 10:58 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/node/disks/directory.rs | 72 +++++++++++++++++++++++++++++++-
 src/tools/systemd.rs             | 11 +++++
 2 files changed, 81 insertions(+), 2 deletions(-)

diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index f05b781f..f8a8d8cb 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -2,7 +2,7 @@ use anyhow::{bail, Error};
 use serde_json::json;
 use ::serde::{Deserialize, Serialize};
 
-use proxmox::api::{api, Permission, RpcEnvironment, RpcEnvironmentType};
+use proxmox::api::{api, Permission, RpcEnvironment, RpcEnvironmentType, HttpError};
 use proxmox::api::section_config::SectionConfigData;
 use proxmox::api::router::Router;
 
@@ -16,6 +16,8 @@ use crate::tools::systemd::{self, types::*};
 use crate::server::WorkerTask;
 
 use crate::api2::types::*;
+use proxmox::api::error::StatusCode;
+use crate::config::datastore::DataStoreConfig;
 
 #[api(
     properties: {
@@ -175,9 +177,75 @@ pub fn create_datastore_disk(
     Ok(upid_str)
 }
 
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            node: {
+                schema: NODE_SCHEMA,
+            },
+            name: {
+                schema: DATASTORE_SCHEMA,
+            },
+        }
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false),
+    },
+)]
+/// Remove a Filesystem mounted under '/mnt/datastore/<name>'.".
+pub fn delete_datastore_disk(
+    name: String,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let path = format!("/mnt/datastore/{}", name);
+    // path of datastore cannot be changed
+    let (config, _) = crate::config::datastore::config()?;
+    let datastores: Vec<DataStoreConfig> = config.convert_to_typed_array("datastore")?;
+    let conflicting_datastore: Option<DataStoreConfig> = datastores.into_iter()
+        .filter(|ds| ds.path == path)
+        .next();
+
+    match conflicting_datastore {
+        Some(conflicting_datastore) =>
+            Err(Error::from(HttpError::new(StatusCode::CONFLICT,
+                                           format!("Can't remove '{}' since it's required by datastore '{}'",
+                                                   conflicting_datastore.path,
+                                                   conflicting_datastore.name)))),
+        None => {
+            // disable systemd mount-unit
+            let mut mount_unit_name = systemd::escape_unit(&path, true);
+            mount_unit_name.push_str(".mount");
+            systemd::disable_unit(mount_unit_name.as_str())?;
+
+            // delete .mount-file
+            let mount_unit_path = format!("/etc/systemd/system/{}", mount_unit_name);
+            let full_path = std::path::Path::new(mount_unit_path.as_str());
+            log::info!("removing {:?}", full_path);
+            std::fs::remove_file(&full_path)?;
+
+            // try to unmount, if that fails tell the user to reboot or unmount manually
+            let mut command = std::process::Command::new("umount");
+            command.arg(path.as_str());
+            match crate::tools::run_command(command, None) {
+                Err(_) => bail!(
+                    "Could not umount '{}' since it is busy. It will stay mounted \
+                    until the next reboot or until unmounted manually!",
+                    path
+                 ),
+                Ok(_) => Ok(())
+            }
+        }
+    }
+}
+
+const ITEM_ROUTER: Router = Router::new()
+    .delete(&API_METHOD_DELETE_DATASTORE_DISK);
+
 pub const ROUTER: Router = Router::new()
     .get(&API_METHOD_LIST_DATASTORE_MOUNTS)
-    .post(&API_METHOD_CREATE_DATASTORE_DISK);
+    .post(&API_METHOD_CREATE_DATASTORE_DISK)
+    .match_all("name", &ITEM_ROUTER);
 
 
 fn create_datastore_mount_unit(
diff --git a/src/tools/systemd.rs b/src/tools/systemd.rs
index 6dde06a3..9a6439de 100644
--- a/src/tools/systemd.rs
+++ b/src/tools/systemd.rs
@@ -83,6 +83,17 @@ pub fn reload_daemon() -> Result<(), Error> {
     Ok(())
 }
 
+pub fn disable_unit(unit: &str) -> Result<(), Error> {
+
+    let mut command = std::process::Command::new("systemctl");
+    command.arg("disable");
+    command.arg(unit);
+
+    crate::tools::run_command(command, None)?;
+
+    Ok(())
+}
+
 pub fn enable_unit(unit: &str) -> Result<(), Error> {
 
     let mut command = std::process::Command::new("systemctl");
-- 
2.20.1





^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v3 2/2] ui/cli: added support for the removal of mount-units
  2020-08-13 10:58 [pbs-devel] [PATCH proxmox-backup v3 0/2] removal of mount-units/directories Hannes Laimer
  2020-08-13 10:58 ` [pbs-devel] [PATCH proxmox-backup v3 1/2] api2/node/../disks/directory: added DELETE endpoint for removal of mount-units Hannes Laimer
@ 2020-08-13 10:58 ` Hannes Laimer
  2020-08-14  5:41   ` Dietmar Maurer
  2020-08-14  8:13   ` Dominik Csapak
  1 sibling, 2 replies; 6+ messages in thread
From: Hannes Laimer @ 2020-08-13 10:58 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/bin/proxmox_backup_manager/disk.rs |  29 +++-
 www/DirectoryList.js                   | 150 ++++++++++--------
 www/Makefile                           |   1 +
 www/window/SafeRemove.js               | 209 +++++++++++++++++++++++++
 4 files changed, 325 insertions(+), 64 deletions(-)
 create mode 100644 www/window/SafeRemove.js

diff --git a/src/bin/proxmox_backup_manager/disk.rs b/src/bin/proxmox_backup_manager/disk.rs
index a93a6f6b..b7eec592 100644
--- a/src/bin/proxmox_backup_manager/disk.rs
+++ b/src/bin/proxmox_backup_manager/disk.rs
@@ -319,6 +319,32 @@ async fn create_datastore_disk(
     Ok(Value::Null)
 }
 
+#[api(
+    input: {
+        properties: {
+            name: {
+                schema: DATASTORE_SCHEMA,
+            },
+        },
+    },
+)]
+/// Remove a Filesystem mounted under '/mnt/datastore/<name>'.".
+async fn delete_datastore_disk(
+    mut param: Value,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Value, Error> {
+
+    param["node"] = "localhost".into();
+
+    let info = &api2::node::disks::directory::API_METHOD_DELETE_DATASTORE_DISK;
+    match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    Ok(Value::Null)
+}
+
 pub fn filesystem_commands() -> CommandLineInterface {
 
     let cmd_def = CliCommandMap::new()
@@ -327,7 +353,8 @@ pub fn filesystem_commands() -> CommandLineInterface {
                 CliCommand::new(&API_METHOD_CREATE_DATASTORE_DISK)
                 .arg_param(&["name"])
                 .completion_cb("disk", complete_disk_name)
-        );
+        ).insert("remove", CliCommand::new(&API_METHOD_DELETE_DATASTORE_DISK)
+                .arg_param(&["name"]));
 
     cmd_def.into()
 }
diff --git a/www/DirectoryList.js b/www/DirectoryList.js
index 00531fd0..6ec91e6a 100644
--- a/www/DirectoryList.js
+++ b/www/DirectoryList.js
@@ -8,83 +8,107 @@ Ext.define('PBS.admin.Directorylist', {
     emptyText: gettext('No Mount-Units found'),
 
     controller: {
-	xclass: 'Ext.app.ViewController',
+        xclass: 'Ext.app.ViewController',
 
-	createDirectory: function() {
-	    let me = this;
-	    Ext.create('PBS.window.CreateDirectory', {
-		listeners: {
-		    destroy: function() {
-			me.reload();
-		    },
-		},
-	    }).show();
-	},
+        createDirectory: function () {
+            console.log(this);
+            let me = this;
+            Ext.create('PBS.window.CreateDirectory', {
+                listeners: {
+                    destroy: function () {
+                        me.reload();
+                    },
+                },
+            }).show();
+        },
 
-	reload: function() {
-	    let me = this;
-	    let store = me.getView().getStore();
-	    store.load();
-	    store.sort();
-	},
+        removeDir: function () {
+            let me = this;
+            let view = me.getView();
+            let rec = view.getSelection();
+            let dialog = Ext.create('PBS.window.SafeDestroy', {
+                url: rec[0].data.path.replace(
+                    '/mnt/datastore/',
+                    '/nodes/localhost/disks/directory/'),
+                item: {type: 'Dir', id: rec[0].data.path.replace('/mnt/datastore/', '')},
+                note: 'Data and partitions on the disk will be left untouched.'
+            });
+            dialog.on('destroy', function() {
+                me.reload();
+            });
+            dialog.show();
+        },
 
-	init: function(view) {
-	    let me = this;
-	    Proxmox.Utils.monStoreErrors(view, view.getStore(), true);
-	    me.reload();
-	},
-    },
+        reload: function () {
+            let me = this;
+            let store = me.getView().getStore();
+            store.load();
+            store.sort();
+        },
 
+        init: function (view) {
+            let me = this;
+            Proxmox.Utils.monStoreErrors(view, view.getStore(), true);
+            me.reload();
+        },
+    },
 
     rootVisible: false,
     useArrows: true,
 
     tbar: [
-	{
-	    text: gettext('Reload'),
-	    iconCls: 'fa fa-refresh',
-	    handler: 'reload',
-	},
-	{
-	    text: gettext('Create') + ': Directory',
-	    handler: 'createDirectory',
-	},
+        {
+            text: gettext('Reload'),
+            iconCls: 'fa fa-refresh',
+            handler: 'reload',
+        },
+        {
+            text: gettext('Create') + ': Directory',
+            handler: 'createDirectory',
+        },
+        {
+            xtype: 'proxmoxButton',
+            text: gettext('Remove'),
+            handler: 'removeDir',
+            disabled: true,
+            iconCls: 'fa fa-trash-o'
+        }
     ],
 
     columns: [
-	{
-	    text: gettext('Path'),
-	    dataIndex: 'path',
-	    flex: 1,
-	},
-	{
-	    header: gettext('Device'),
-	    flex: 1,
-	    dataIndex: 'device',
-	},
-	{
-	    header: gettext('Filesystem'),
-	    width: 100,
-	    dataIndex: 'filesystem',
-	},
-	{
-	    header: gettext('Options'),
-	    width: 100,
-	    dataIndex: 'options',
-	},
-	{
-	    header: gettext('Unit File'),
-	    hidden: true,
-	    dataIndex: 'unitfile',
-	},
+        {
+            text: gettext('Path'),
+            dataIndex: 'path',
+            flex: 1,
+        },
+        {
+            header: gettext('Device'),
+            flex: 1,
+            dataIndex: 'device',
+        },
+        {
+            header: gettext('Filesystem'),
+            width: 100,
+            dataIndex: 'filesystem',
+        },
+        {
+            header: gettext('Options'),
+            width: 100,
+            dataIndex: 'options',
+        },
+        {
+            header: gettext('Unit File'),
+            hidden: true,
+            dataIndex: 'unitfile',
+        },
     ],
 
     store: {
-	fields: ['path', 'device', 'filesystem', 'options', 'unitfile'],
-	proxy: {
-	    type: 'proxmox',
-	    url: '/api2/json/nodes/localhost/disks/directory',
-	},
-	sorters: 'path',
+        fields: ['path', 'device', 'filesystem', 'options', 'unitfile'],
+        proxy: {
+            type: 'proxmox',
+            url: '/api2/json/nodes/localhost/disks/directory',
+        },
+        sorters: 'path',
     },
 });
diff --git a/www/Makefile b/www/Makefile
index edce8cb3..57db54ee 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -23,6 +23,7 @@ JSSRC=							\
 	window/SyncJobEdit.js				\
 	window/ACLEdit.js				\
 	window/DataStoreEdit.js				\
+	window/SafeRemove.js				\
 	window/CreateDirectory.js			\
 	window/ZFSCreate.js				\
 	window/FileBrowser.js				\
diff --git a/www/window/SafeRemove.js b/www/window/SafeRemove.js
new file mode 100644
index 00000000..fdbefeae
--- /dev/null
+++ b/www/window/SafeRemove.js
@@ -0,0 +1,209 @@
+/* Popup a message window
+ * where the user has to manually enter the resource ID
+ * to enable the destroy button
+ * (mostly taken from PVE-Manager(git.proxmox.com/git/pve-manager.git))
+ */
+Ext.define('PBS.window.SafeDestroy', {
+    extend: 'Ext.window.Window',
+
+    title: gettext('Confirm'),
+    modal: true,
+    buttonAlign: 'center',
+    bodyPadding: 10,
+    width: 450,
+    layout: {type: 'hbox'},
+    defaultFocus: 'confirmField',
+    showProgress: false,
+
+    config: {
+        item: {
+            id: undefined,
+            type: undefined
+        },
+        url: undefined,
+        note: undefined,
+        params: {}
+    },
+
+    getParams: function () {
+        var me = this;
+        var wipeCheckbox = me.lookupReference('wipeCheckbox');
+        if (wipeCheckbox.checked) {
+            me.params.wipe = 1;
+        }
+        if (Ext.Object.isEmpty(me.params)) {
+            return '';
+        }
+        return '?' + Ext.Object.toQueryString(me.params);
+    },
+
+    controller: {
+
+        xclass: 'Ext.app.ViewController',
+
+        control: {
+            'field[name=confirm]': {
+                change: function (f, value) {
+                    var view = this.getView();
+                    var removeButton = this.lookupReference('removeButton');
+                    if (value === view.getItem().id.toString()) {
+                        removeButton.enable();
+                    } else {
+                        removeButton.disable();
+                    }
+                },
+                specialkey: function (field, event) {
+                    var removeButton = this.lookupReference('removeButton');
+                    if (!removeButton.isDisabled() && event.getKey() == event.ENTER) {
+                        removeButton.fireEvent('click', removeButton, event);
+                    }
+                }
+            },
+            'button[reference=removeButton]': {
+                click: function () {
+                    var view = this.getView();
+                    Proxmox.Utils.API2Request({
+                        url: view.getUrl() + view.getParams(),
+                        method: 'DELETE',
+                        waitMsgTarget: view,
+                        failure: function (response, opts) {
+                            view.close();
+                            Ext.Msg.alert('Error', response.htmlStatus);
+                        },
+                        success: function (response, options) {
+                            var hasProgressBar = view.showProgress &&
+                            response.result.data ? true : false;
+
+                            if (hasProgressBar) {
+                                // stay around so we can trigger our close events
+                                // when background action is completed
+                                view.hide();
+
+                                var upid = response.result.data;
+                                var win = Ext.create('Proxmox.window.TaskProgress', {
+                                    upid: upid,
+                                    listeners: {
+                                        destroy: function () {
+                                            view.close();
+                                        }
+                                    }
+                                });
+                                win.show();
+                            } else {
+                                view.close();
+                            }
+                        }
+                    });
+                }
+            }
+        }
+    },
+
+    items: [
+        {
+            xtype: 'component',
+            cls: [Ext.baseCSSPrefix + 'message-box-icon',
+                Ext.baseCSSPrefix + 'message-box-warning',
+                Ext.baseCSSPrefix + 'dlg-icon']
+        },
+        {
+            xtype: 'container',
+            flex: 1,
+            layout: {
+                type: 'vbox',
+                align: 'stretch'
+            },
+            items: [
+                {
+                    xtype: 'component',
+                    reference: 'messageCmp'
+                },
+                {
+                    itemId: 'confirmField',
+                    reference: 'confirmField',
+                    xtype: 'textfield',
+                    name: 'confirm',
+                    labelWidth: 300,
+                    hideTrigger: true,
+                    allowBlank: false
+                },
+                {
+                    xtype: 'proxmoxcheckbox',
+                    name: 'wipe',
+                    reference: 'wipeCheckbox',
+                    boxLabel: gettext('Wipe'),
+                    checked: false,
+                    autoEl: {
+                        tag: 'div',
+                        'data-qtip': gettext('Wipe disk after the removal of mountpoint')
+                    }
+                },
+                {
+                    xtype: 'container',
+                    flex: 1,
+                    layout: {
+                        type: 'vbox',
+                        align: 'middle'
+                    },
+                    height: 25,
+                    items: [
+                        {
+                            xtype: 'component',
+                            reference: 'noteCmp'
+                        },
+                    ]
+                },
+            ]
+        }
+    ],
+    buttons: [
+        {
+            reference: 'removeButton',
+            text: gettext('Remove'),
+            disabled: true
+        }
+    ],
+
+    initComponent: function () {
+        var me = this;
+
+        me.callParent();
+
+        var item = me.getItem();
+
+        if (!Ext.isDefined(item.id)) {
+            throw "no ID specified";
+        }
+
+        if (!Ext.isDefined(item.type)) {
+            throw "no Disk type specified";
+        }
+
+        var messageCmp = me.lookupReference('messageCmp');
+        var noteCmp = me.lookupReference('noteCmp');
+        var msg;
+
+        if (item.type === 'Dir') {
+            msg = `Directory ${item.id} - Remove`
+        } else {
+            throw "unknown item type specified";
+        }
+
+        if(me.getNote()) {
+            noteCmp.setHtml(`<small>${me.getNote()}</small>`);
+        }
+
+        messageCmp.setHtml(msg);
+
+        if (item.type === 'Dir') {
+            let wipeCheckbox = me.lookupReference('wipeCheckbox');
+            wipeCheckbox.setDisabled(true);
+            wipeCheckbox.setHidden(true);
+        }
+
+        var confirmField = me.lookupReference('confirmField');
+        msg = gettext('Please enter the ID to confirm') +
+            ' (' + item.id + ')';
+        confirmField.setFieldLabel(msg);
+    }
+});
-- 
2.20.1





^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pbs-devel] applied: [PATCH proxmox-backup v3 1/2] api2/node/../disks/directory: added DELETE endpoint for removal of mount-units
  2020-08-13 10:58 ` [pbs-devel] [PATCH proxmox-backup v3 1/2] api2/node/../disks/directory: added DELETE endpoint for removal of mount-units Hannes Laimer
@ 2020-08-14  5:38   ` Dietmar Maurer
  0 siblings, 0 replies; 6+ messages in thread
From: Dietmar Maurer @ 2020-08-14  5:38 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Hannes Laimer

applied with some cleanup on top - see inline comments

> On 08/13/2020 12:58 PM Hannes Laimer <h.laimer@proxmox.com> wrote:
> 
>  
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/api2/node/disks/directory.rs | 72 +++++++++++++++++++++++++++++++-
>  src/tools/systemd.rs             | 11 +++++
>  2 files changed, 81 insertions(+), 2 deletions(-)
> 
> diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
> index f05b781f..f8a8d8cb 100644
> --- a/src/api2/node/disks/directory.rs
> +++ b/src/api2/node/disks/directory.rs
> @@ -2,7 +2,7 @@ use anyhow::{bail, Error};
>  use serde_json::json;
>  use ::serde::{Deserialize, Serialize};
>  
> -use proxmox::api::{api, Permission, RpcEnvironment, RpcEnvironmentType};
> +use proxmox::api::{api, Permission, RpcEnvironment, RpcEnvironmentType, HttpError};
>  use proxmox::api::section_config::SectionConfigData;
>  use proxmox::api::router::Router;
>  
> @@ -16,6 +16,8 @@ use crate::tools::systemd::{self, types::*};
>  use crate::server::WorkerTask;
>  
>  use crate::api2::types::*;
> +use proxmox::api::error::StatusCode;
> +use crate::config::datastore::DataStoreConfig;
>  
>  #[api(
>      properties: {
> @@ -175,9 +177,75 @@ pub fn create_datastore_disk(
>      Ok(upid_str)
>  }
>  
> +#[api(
> +    protected: true,
> +    input: {
> +        properties: {
> +            node: {
> +                schema: NODE_SCHEMA,
> +            },
> +            name: {
> +                schema: DATASTORE_SCHEMA,
> +            },
> +        }
> +    },
> +    access: {
> +        permission: &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false),
> +    },
> +)]
> +/// Remove a Filesystem mounted under '/mnt/datastore/<name>'.".
> +pub fn delete_datastore_disk(
> +    name: String,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<(), Error> {
> +    let path = format!("/mnt/datastore/{}", name);
> +    // path of datastore cannot be changed
> +    let (config, _) = crate::config::datastore::config()?;
> +    let datastores: Vec<DataStoreConfig> = config.convert_to_typed_array("datastore")?;
> +    let conflicting_datastore: Option<DataStoreConfig> = datastores.into_iter()
> +        .filter(|ds| ds.path == path)
> +        .next();
> +
> +    match conflicting_datastore {
> +        Some(conflicting_datastore) =>
> +            Err(Error::from(HttpError::new(StatusCode::CONFLICT,
> +                                           format!("Can't remove '{}' since it's required by datastore '{}'",
> +                                                   conflicting_datastore.path,
> +                                                   conflicting_datastore.name)))),

What do you use HttpError here? Instead, we can simply use bail! as everywhere else.

Also, there is a proxmox::http_bail! macro to simplify above code.

> +        None => {
> +            // disable systemd mount-unit
> +            let mut mount_unit_name = systemd::escape_unit(&path, true);
> +            mount_unit_name.push_str(".mount");
> +            systemd::disable_unit(mount_unit_name.as_str())?;

You ca replace that .as_str() by simply using &mount_unit_name

> +
> +            // delete .mount-file
> +            let mount_unit_path = format!("/etc/systemd/system/{}", mount_unit_name);
> +            let full_path = std::path::Path::new(mount_unit_path.as_str());

same here

> +            log::info!("removing {:?}", full_path);
> +            std::fs::remove_file(&full_path)?;
> +
> +            // try to unmount, if that fails tell the user to reboot or unmount manually
> +            let mut command = std::process::Command::new("umount");
> +            command.arg(path.as_str());

and here

> +            match crate::tools::run_command(command, None) {
> +                Err(_) => bail!(
> +                    "Could not umount '{}' since it is busy. It will stay mounted \
> +                    until the next reboot or until unmounted manually!",
> +                    path
> +                 ),
> +                Ok(_) => Ok(())
> +            }
> +        }
> +    }
> +}
> +
> +const ITEM_ROUTER: Router = Router::new()
> +    .delete(&API_METHOD_DELETE_DATASTORE_DISK);
> +
>  pub const ROUTER: Router = Router::new()
>      .get(&API_METHOD_LIST_DATASTORE_MOUNTS)
> -    .post(&API_METHOD_CREATE_DATASTORE_DISK);
> +    .post(&API_METHOD_CREATE_DATASTORE_DISK)
> +    .match_all("name", &ITEM_ROUTER);
>  
>  
>  fn create_datastore_mount_unit(
> diff --git a/src/tools/systemd.rs b/src/tools/systemd.rs
> index 6dde06a3..9a6439de 100644
> --- a/src/tools/systemd.rs
> +++ b/src/tools/systemd.rs
> @@ -83,6 +83,17 @@ pub fn reload_daemon() -> Result<(), Error> {
>      Ok(())
>  }
>  
> +pub fn disable_unit(unit: &str) -> Result<(), Error> {
> +
> +    let mut command = std::process::Command::new("systemctl");
> +    command.arg("disable");
> +    command.arg(unit);
> +
> +    crate::tools::run_command(command, None)?;
> +
> +    Ok(())
> +}
> +
>  pub fn enable_unit(unit: &str) -> Result<(), Error> {
>  
>      let mut command = std::process::Command::new("systemctl");
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v3 2/2] ui/cli: added support for the removal of mount-units
  2020-08-13 10:58 ` [pbs-devel] [PATCH proxmox-backup v3 2/2] ui/cli: added support for the " Hannes Laimer
@ 2020-08-14  5:41   ` Dietmar Maurer
  2020-08-14  8:13   ` Dominik Csapak
  1 sibling, 0 replies; 6+ messages in thread
From: Dietmar Maurer @ 2020-08-14  5:41 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Hannes Laimer

This patch contain many white space changes. If they are required, please sent them
as sparate patch. I would also prefer the CLI code a separate patch.

> On 08/13/2020 12:58 PM Hannes Laimer <h.laimer@proxmox.com> wrote:
> 
>  
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/bin/proxmox_backup_manager/disk.rs |  29 +++-
>  www/DirectoryList.js                   | 150 ++++++++++--------
>  www/Makefile                           |   1 +
>  www/window/SafeRemove.js               | 209 +++++++++++++++++++++++++
>  4 files changed, 325 insertions(+), 64 deletions(-)
>  create mode 100644 www/window/SafeRemove.js
> 
> diff --git a/src/bin/proxmox_backup_manager/disk.rs b/src/bin/proxmox_backup_manager/disk.rs
> index a93a6f6b..b7eec592 100644
> --- a/src/bin/proxmox_backup_manager/disk.rs
> +++ b/src/bin/proxmox_backup_manager/disk.rs
> @@ -319,6 +319,32 @@ async fn create_datastore_disk(
>      Ok(Value::Null)
>  }
>  
> +#[api(
> +    input: {
> +        properties: {
> +            name: {
> +                schema: DATASTORE_SCHEMA,
> +            },
> +        },
> +    },
> +)]
> +/// Remove a Filesystem mounted under '/mnt/datastore/<name>'.".
> +async fn delete_datastore_disk(
> +    mut param: Value,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Value, Error> {
> +
> +    param["node"] = "localhost".into();
> +
> +    let info = &api2::node::disks::directory::API_METHOD_DELETE_DATASTORE_DISK;
> +    match info.handler {
> +        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
> +        _ => unreachable!(),
> +    };
> +
> +    Ok(Value::Null)
> +}
> +
>  pub fn filesystem_commands() -> CommandLineInterface {
>  
>      let cmd_def = CliCommandMap::new()
> @@ -327,7 +353,8 @@ pub fn filesystem_commands() -> CommandLineInterface {
>                  CliCommand::new(&API_METHOD_CREATE_DATASTORE_DISK)
>                  .arg_param(&["name"])
>                  .completion_cb("disk", complete_disk_name)
> -        );
> +        ).insert("remove", CliCommand::new(&API_METHOD_DELETE_DATASTORE_DISK)
> +                .arg_param(&["name"]));
>  
>      cmd_def.into()
>  }
> diff --git a/www/DirectoryList.js b/www/DirectoryList.js
> index 00531fd0..6ec91e6a 100644
> --- a/www/DirectoryList.js
> +++ b/www/DirectoryList.js
> @@ -8,83 +8,107 @@ Ext.define('PBS.admin.Directorylist', {
>      emptyText: gettext('No Mount-Units found'),
>  
>      controller: {
> -	xclass: 'Ext.app.ViewController',
> +        xclass: 'Ext.app.ViewController',
>  
> -	createDirectory: function() {
> -	    let me = this;
> -	    Ext.create('PBS.window.CreateDirectory', {
> -		listeners: {
> -		    destroy: function() {
> -			me.reload();
> -		    },
> -		},
> -	    }).show();
> -	},
> +        createDirectory: function () {
> +            console.log(this);
> +            let me = this;
> +            Ext.create('PBS.window.CreateDirectory', {
> +                listeners: {
> +                    destroy: function () {
> +                        me.reload();
> +                    },
> +                },
> +            }).show();
> +        },
>  
> -	reload: function() {
> -	    let me = this;
> -	    let store = me.getView().getStore();
> -	    store.load();
> -	    store.sort();
> -	},
> +        removeDir: function () {
> +            let me = this;
> +            let view = me.getView();
> +            let rec = view.getSelection();
> +            let dialog = Ext.create('PBS.window.SafeDestroy', {
> +                url: rec[0].data.path.replace(
> +                    '/mnt/datastore/',
> +                    '/nodes/localhost/disks/directory/'),
> +                item: {type: 'Dir', id: rec[0].data.path.replace('/mnt/datastore/', '')},
> +                note: 'Data and partitions on the disk will be left untouched.'
> +            });
> +            dialog.on('destroy', function() {
> +                me.reload();
> +            });
> +            dialog.show();
> +        },
>  
> -	init: function(view) {
> -	    let me = this;
> -	    Proxmox.Utils.monStoreErrors(view, view.getStore(), true);
> -	    me.reload();
> -	},
> -    },
> +        reload: function () {
> +            let me = this;
> +            let store = me.getView().getStore();
> +            store.load();
> +            store.sort();
> +        },
>  
> +        init: function (view) {
> +            let me = this;
> +            Proxmox.Utils.monStoreErrors(view, view.getStore(), true);
> +            me.reload();
> +        },
> +    },
>  
>      rootVisible: false,
>      useArrows: true,
>  
>      tbar: [
> -	{
> -	    text: gettext('Reload'),
> -	    iconCls: 'fa fa-refresh',
> -	    handler: 'reload',
> -	},
> -	{
> -	    text: gettext('Create') + ': Directory',
> -	    handler: 'createDirectory',
> -	},
> +        {
> +            text: gettext('Reload'),
> +            iconCls: 'fa fa-refresh',
> +            handler: 'reload',
> +        },
> +        {
> +            text: gettext('Create') + ': Directory',
> +            handler: 'createDirectory',
> +        },
> +        {
> +            xtype: 'proxmoxButton',
> +            text: gettext('Remove'),
> +            handler: 'removeDir',
> +            disabled: true,
> +            iconCls: 'fa fa-trash-o'
> +        }
>      ],
>  
>      columns: [
> -	{
> -	    text: gettext('Path'),
> -	    dataIndex: 'path',
> -	    flex: 1,
> -	},
> -	{
> -	    header: gettext('Device'),
> -	    flex: 1,
> -	    dataIndex: 'device',
> -	},
> -	{
> -	    header: gettext('Filesystem'),
> -	    width: 100,
> -	    dataIndex: 'filesystem',
> -	},
> -	{
> -	    header: gettext('Options'),
> -	    width: 100,
> -	    dataIndex: 'options',
> -	},
> -	{
> -	    header: gettext('Unit File'),
> -	    hidden: true,
> -	    dataIndex: 'unitfile',
> -	},
> +        {
> +            text: gettext('Path'),
> +            dataIndex: 'path',
> +            flex: 1,
> +        },
> +        {
> +            header: gettext('Device'),
> +            flex: 1,
> +            dataIndex: 'device',
> +        },
> +        {
> +            header: gettext('Filesystem'),
> +            width: 100,
> +            dataIndex: 'filesystem',
> +        },
> +        {
> +            header: gettext('Options'),
> +            width: 100,
> +            dataIndex: 'options',
> +        },
> +        {
> +            header: gettext('Unit File'),
> +            hidden: true,
> +            dataIndex: 'unitfile',
> +        },
>      ],
>  
>      store: {
> -	fields: ['path', 'device', 'filesystem', 'options', 'unitfile'],
> -	proxy: {
> -	    type: 'proxmox',
> -	    url: '/api2/json/nodes/localhost/disks/directory',
> -	},
> -	sorters: 'path',
> +        fields: ['path', 'device', 'filesystem', 'options', 'unitfile'],
> +        proxy: {
> +            type: 'proxmox',
> +            url: '/api2/json/nodes/localhost/disks/directory',
> +        },
> +        sorters: 'path',
>      },
>  });
> diff --git a/www/Makefile b/www/Makefile
> index edce8cb3..57db54ee 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -23,6 +23,7 @@ JSSRC=							\
>  	window/SyncJobEdit.js				\
>  	window/ACLEdit.js				\
>  	window/DataStoreEdit.js				\
> +	window/SafeRemove.js				\
>  	window/CreateDirectory.js			\
>  	window/ZFSCreate.js				\
>  	window/FileBrowser.js				\
> diff --git a/www/window/SafeRemove.js b/www/window/SafeRemove.js
> new file mode 100644
> index 00000000..fdbefeae
> --- /dev/null
> +++ b/www/window/SafeRemove.js
> @@ -0,0 +1,209 @@
> +/* Popup a message window
> + * where the user has to manually enter the resource ID
> + * to enable the destroy button
> + * (mostly taken from PVE-Manager(git.proxmox.com/git/pve-manager.git))
> + */
> +Ext.define('PBS.window.SafeDestroy', {
> +    extend: 'Ext.window.Window',
> +
> +    title: gettext('Confirm'),
> +    modal: true,
> +    buttonAlign: 'center',
> +    bodyPadding: 10,
> +    width: 450,
> +    layout: {type: 'hbox'},
> +    defaultFocus: 'confirmField',
> +    showProgress: false,
> +
> +    config: {
> +        item: {
> +            id: undefined,
> +            type: undefined
> +        },
> +        url: undefined,
> +        note: undefined,
> +        params: {}
> +    },
> +
> +    getParams: function () {
> +        var me = this;
> +        var wipeCheckbox = me.lookupReference('wipeCheckbox');
> +        if (wipeCheckbox.checked) {
> +            me.params.wipe = 1;
> +        }
> +        if (Ext.Object.isEmpty(me.params)) {
> +            return '';
> +        }
> +        return '?' + Ext.Object.toQueryString(me.params);
> +    },
> +
> +    controller: {
> +
> +        xclass: 'Ext.app.ViewController',
> +
> +        control: {
> +            'field[name=confirm]': {
> +                change: function (f, value) {
> +                    var view = this.getView();
> +                    var removeButton = this.lookupReference('removeButton');
> +                    if (value === view.getItem().id.toString()) {
> +                        removeButton.enable();
> +                    } else {
> +                        removeButton.disable();
> +                    }
> +                },
> +                specialkey: function (field, event) {
> +                    var removeButton = this.lookupReference('removeButton');
> +                    if (!removeButton.isDisabled() && event.getKey() == event.ENTER) {
> +                        removeButton.fireEvent('click', removeButton, event);
> +                    }
> +                }
> +            },
> +            'button[reference=removeButton]': {
> +                click: function () {
> +                    var view = this.getView();
> +                    Proxmox.Utils.API2Request({
> +                        url: view.getUrl() + view.getParams(),
> +                        method: 'DELETE',
> +                        waitMsgTarget: view,
> +                        failure: function (response, opts) {
> +                            view.close();
> +                            Ext.Msg.alert('Error', response.htmlStatus);
> +                        },
> +                        success: function (response, options) {
> +                            var hasProgressBar = view.showProgress &&
> +                            response.result.data ? true : false;
> +
> +                            if (hasProgressBar) {
> +                                // stay around so we can trigger our close events
> +                                // when background action is completed
> +                                view.hide();
> +
> +                                var upid = response.result.data;
> +                                var win = Ext.create('Proxmox.window.TaskProgress', {
> +                                    upid: upid,
> +                                    listeners: {
> +                                        destroy: function () {
> +                                            view.close();
> +                                        }
> +                                    }
> +                                });
> +                                win.show();
> +                            } else {
> +                                view.close();
> +                            }
> +                        }
> +                    });
> +                }
> +            }
> +        }
> +    },
> +
> +    items: [
> +        {
> +            xtype: 'component',
> +            cls: [Ext.baseCSSPrefix + 'message-box-icon',
> +                Ext.baseCSSPrefix + 'message-box-warning',
> +                Ext.baseCSSPrefix + 'dlg-icon']
> +        },
> +        {
> +            xtype: 'container',
> +            flex: 1,
> +            layout: {
> +                type: 'vbox',
> +                align: 'stretch'
> +            },
> +            items: [
> +                {
> +                    xtype: 'component',
> +                    reference: 'messageCmp'
> +                },
> +                {
> +                    itemId: 'confirmField',
> +                    reference: 'confirmField',
> +                    xtype: 'textfield',
> +                    name: 'confirm',
> +                    labelWidth: 300,
> +                    hideTrigger: true,
> +                    allowBlank: false
> +                },
> +                {
> +                    xtype: 'proxmoxcheckbox',
> +                    name: 'wipe',
> +                    reference: 'wipeCheckbox',
> +                    boxLabel: gettext('Wipe'),
> +                    checked: false,
> +                    autoEl: {
> +                        tag: 'div',
> +                        'data-qtip': gettext('Wipe disk after the removal of mountpoint')
> +                    }
> +                },
> +                {
> +                    xtype: 'container',
> +                    flex: 1,
> +                    layout: {
> +                        type: 'vbox',
> +                        align: 'middle'
> +                    },
> +                    height: 25,
> +                    items: [
> +                        {
> +                            xtype: 'component',
> +                            reference: 'noteCmp'
> +                        },
> +                    ]
> +                },
> +            ]
> +        }
> +    ],
> +    buttons: [
> +        {
> +            reference: 'removeButton',
> +            text: gettext('Remove'),
> +            disabled: true
> +        }
> +    ],
> +
> +    initComponent: function () {
> +        var me = this;
> +
> +        me.callParent();
> +
> +        var item = me.getItem();
> +
> +        if (!Ext.isDefined(item.id)) {
> +            throw "no ID specified";
> +        }
> +
> +        if (!Ext.isDefined(item.type)) {
> +            throw "no Disk type specified";
> +        }
> +
> +        var messageCmp = me.lookupReference('messageCmp');
> +        var noteCmp = me.lookupReference('noteCmp');
> +        var msg;
> +
> +        if (item.type === 'Dir') {
> +            msg = `Directory ${item.id} - Remove`
> +        } else {
> +            throw "unknown item type specified";
> +        }
> +
> +        if(me.getNote()) {
> +            noteCmp.setHtml(`<small>${me.getNote()}</small>`);
> +        }
> +
> +        messageCmp.setHtml(msg);
> +
> +        if (item.type === 'Dir') {
> +            let wipeCheckbox = me.lookupReference('wipeCheckbox');
> +            wipeCheckbox.setDisabled(true);
> +            wipeCheckbox.setHidden(true);
> +        }
> +
> +        var confirmField = me.lookupReference('confirmField');
> +        msg = gettext('Please enter the ID to confirm') +
> +            ' (' + item.id + ')';
> +        confirmField.setFieldLabel(msg);
> +    }
> +});
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v3 2/2] ui/cli: added support for the removal of mount-units
  2020-08-13 10:58 ` [pbs-devel] [PATCH proxmox-backup v3 2/2] ui/cli: added support for the " Hannes Laimer
  2020-08-14  5:41   ` Dietmar Maurer
@ 2020-08-14  8:13   ` Dominik Csapak
  1 sibling, 0 replies; 6+ messages in thread
From: Dominik Csapak @ 2020-08-14  8:13 UTC (permalink / raw)
  To: pbs-devel

aside from dietmars comments, ui code looks mostly ok
some comments inline

On 8/13/20 12:58 PM, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   src/bin/proxmox_backup_manager/disk.rs |  29 +++-
>   www/DirectoryList.js                   | 150 ++++++++++--------
>   www/Makefile                           |   1 +
>   www/window/SafeRemove.js               | 209 +++++++++++++++++++++++++
>   4 files changed, 325 insertions(+), 64 deletions(-)
>   create mode 100644 www/window/SafeRemove.js
> 
> diff --git a/src/bin/proxmox_backup_manager/disk.rs b/src/bin/proxmox_backup_manager/disk.rs
> index a93a6f6b..b7eec592 100644
> --- a/src/bin/proxmox_backup_manager/disk.rs
> +++ b/src/bin/proxmox_backup_manager/disk.rs
> @@ -319,6 +319,32 @@ async fn create_datastore_disk(
>       Ok(Value::Null)
>   }
>   
> +#[api(
> +    input: {
> +        properties: {
> +            name: {
> +                schema: DATASTORE_SCHEMA,
> +            },
> +        },
> +    },
> +)]
> +/// Remove a Filesystem mounted under '/mnt/datastore/<name>'.".
> +async fn delete_datastore_disk(
> +    mut param: Value,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Value, Error> {
> +
> +    param["node"] = "localhost".into();
> +
> +    let info = &api2::node::disks::directory::API_METHOD_DELETE_DATASTORE_DISK;
> +    match info.handler {
> +        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
> +        _ => unreachable!(),
> +    };
> +
> +    Ok(Value::Null)
> +}
> +
>   pub fn filesystem_commands() -> CommandLineInterface {
>   
>       let cmd_def = CliCommandMap::new()
> @@ -327,7 +353,8 @@ pub fn filesystem_commands() -> CommandLineInterface {
>                   CliCommand::new(&API_METHOD_CREATE_DATASTORE_DISK)
>                   .arg_param(&["name"])
>                   .completion_cb("disk", complete_disk_name)
> -        );
> +        ).insert("remove", CliCommand::new(&API_METHOD_DELETE_DATASTORE_DISK)
> +                .arg_param(&["name"]));
>   
>       cmd_def.into()
>   }
> diff --git a/www/DirectoryList.js b/www/DirectoryList.js
> index 00531fd0..6ec91e6a 100644
> --- a/www/DirectoryList.js
> +++ b/www/DirectoryList.js
> @@ -8,83 +8,107 @@ Ext.define('PBS.admin.Directorylist', {
>       emptyText: gettext('No Mount-Units found'),
>   
>       controller: {
> -	xclass: 'Ext.app.ViewController',
> +        xclass: 'Ext.app.ViewController',
>   
> -	createDirectory: function() {
> -	    let me = this;
> -	    Ext.create('PBS.window.CreateDirectory', {
> -		listeners: {
> -		    destroy: function() {
> -			me.reload();
> -		    },
> -		},
> -	    }).show();
> -	},
> +        createDirectory: function () {
> +            console.log(this);

i guess this is leftover?

> +            let me = this;
> +            Ext.create('PBS.window.CreateDirectory', {
> +                listeners: {
> +                    destroy: function () {
> +                        me.reload();
> +                    },
> +                },
> +            }).show();
> +        },
>   
> -	reload: function() {
> -	    let me = this;
> -	    let store = me.getView().getStore();
> -	    store.load();
> -	    store.sort();
> -	},
> +        removeDir: function () {
> +            let me = this;
> +            let view = me.getView();
> +            let rec = view.getSelection();
> +            let dialog = Ext.create('PBS.window.SafeDestroy', {
> +                url: rec[0].data.path.replace(
> +                    '/mnt/datastore/',
> +                    '/nodes/localhost/disks/directory/'),
> +                item: {type: 'Dir', id: rec[0].data.path.replace('/mnt/datastore/', '')},

that seems brittle... would it not be better to return the 'id' from the 
api instead of modifying the path in the frontend?

> +                note: 'Data and partitions on the disk will be left untouched.'

i think it would be better to put that text in a 'gettext' so it can be 
translated

> +            });
> +            dialog.on('destroy', function() {
> +                me.reload();
> +            });
> +            dialog.show();

i would here prefer the same codestyle as in createDirectory, so

Ext.create('', {
     ...
     listeners: {
	destroy: ...
     },
}).show();

> +        },
>   
> -	init: function(view) {
> -	    let me = this;
> -	    Proxmox.Utils.monStoreErrors(view, view.getStore(), true);
> -	    me.reload();
> -	},
> -    },
> +        reload: function () {
> +            let me = this;
> +            let store = me.getView().getStore();
> +            store.load();
> +            store.sort();
> +        },
>   
> +        init: function (view) {
> +            let me = this;
> +            Proxmox.Utils.monStoreErrors(view, view.getStore(), true);
> +            me.reload();
> +        },
> +    },
>   
>       rootVisible: false,
>       useArrows: true,
>   
>       tbar: [
> -	{
> -	    text: gettext('Reload'),
> -	    iconCls: 'fa fa-refresh',
> -	    handler: 'reload',
> -	},
> -	{
> -	    text: gettext('Create') + ': Directory',
> -	    handler: 'createDirectory',
> -	},
> +        {
> +            text: gettext('Reload'),
> +            iconCls: 'fa fa-refresh',
> +            handler: 'reload',
> +        },
> +        {
> +            text: gettext('Create') + ': Directory',
> +            handler: 'createDirectory',
> +        },
> +        {
> +            xtype: 'proxmoxButton',
> +            text: gettext('Remove'),
> +            handler: 'removeDir',
> +            disabled: true,
> +            iconCls: 'fa fa-trash-o'
> +        }
>       ],
>   
>       columns: [
> -	{
> -	    text: gettext('Path'),
> -	    dataIndex: 'path',
> -	    flex: 1,
> -	},
> -	{
> -	    header: gettext('Device'),
> -	    flex: 1,
> -	    dataIndex: 'device',
> -	},
> -	{
> -	    header: gettext('Filesystem'),
> -	    width: 100,
> -	    dataIndex: 'filesystem',
> -	},
> -	{
> -	    header: gettext('Options'),
> -	    width: 100,
> -	    dataIndex: 'options',
> -	},
> -	{
> -	    header: gettext('Unit File'),
> -	    hidden: true,
> -	    dataIndex: 'unitfile',
> -	},
> +        {
> +            text: gettext('Path'),
> +            dataIndex: 'path',
> +            flex: 1,
> +        },
> +        {
> +            header: gettext('Device'),
> +            flex: 1,
> +            dataIndex: 'device',
> +        },
> +        {
> +            header: gettext('Filesystem'),
> +            width: 100,
> +            dataIndex: 'filesystem',
> +        },
> +        {
> +            header: gettext('Options'),
> +            width: 100,
> +            dataIndex: 'options',
> +        },
> +        {
> +            header: gettext('Unit File'),
> +            hidden: true,
> +            dataIndex: 'unitfile',
> +        },
>       ],
>   
>       store: {
> -	fields: ['path', 'device', 'filesystem', 'options', 'unitfile'],
> -	proxy: {
> -	    type: 'proxmox',
> -	    url: '/api2/json/nodes/localhost/disks/directory',
> -	},
> -	sorters: 'path',
> +        fields: ['path', 'device', 'filesystem', 'options', 'unitfile'],
> +        proxy: {
> +            type: 'proxmox',
> +            url: '/api2/json/nodes/localhost/disks/directory',
> +        },
> +        sorters: 'path',
>       },
>   });
> diff --git a/www/Makefile b/www/Makefile
> index edce8cb3..57db54ee 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -23,6 +23,7 @@ JSSRC=							\
>   	window/SyncJobEdit.js				\
>   	window/ACLEdit.js				\
>   	window/DataStoreEdit.js				\
> +	window/SafeRemove.js				\
>   	window/CreateDirectory.js			\
>   	window/ZFSCreate.js				\
>   	window/FileBrowser.js				\
> diff --git a/www/window/SafeRemove.js b/www/window/SafeRemove.js
> new file mode 100644
> index 00000000..fdbefeae
> --- /dev/null
> +++ b/www/window/SafeRemove.js
> @@ -0,0 +1,209 @@
> +/* Popup a message window
> + * where the user has to manually enter the resource ID
> + * to enable the destroy button
> + * (mostly taken from PVE-Manager(git.proxmox.com/git/pve-manager.git))
> + */

i would rather prefer to refactor that safedestroy window into
the widget-toolkit

aside from handling the new type is there any reason
not to refactor and reuse?

i diffed over the two files and mostly
saw the different checkbox (which can be handled by showing/hiding the
correct one, or have the text and parameter as arguments)
and the note (which can also be shown/hidden by type i guess)

> +Ext.define('PBS.window.SafeDestroy', {
> +    extend: 'Ext.window.Window',
> +
> +    title: gettext('Confirm'),
> +    modal: true,
> +    buttonAlign: 'center',
> +    bodyPadding: 10,
> +    width: 450,
> +    layout: {type: 'hbox'},
> +    defaultFocus: 'confirmField',
> +    showProgress: false,
> +
> +    config: {
> +        item: {
> +            id: undefined,
> +            type: undefined
> +        },
> +        url: undefined,
> +        note: undefined,
> +        params: {}
> +    },
> +
> +    getParams: function () {
> +        var me = this;
> +        var wipeCheckbox = me.lookupReference('wipeCheckbox');
> +        if (wipeCheckbox.checked) {
> +            me.params.wipe = 1;
> +        }
> +        if (Ext.Object.isEmpty(me.params)) {
> +            return '';
> +        }
> +        return '?' + Ext.Object.toQueryString(me.params);
> +    },
> +
> +    controller: {
> +
> +        xclass: 'Ext.app.ViewController',
> +
> +        control: {
> +            'field[name=confirm]': {
> +                change: function (f, value) {
> +                    var view = this.getView();
> +                    var removeButton = this.lookupReference('removeButton');
> +                    if (value === view.getItem().id.toString()) {
> +                        removeButton.enable();
> +                    } else {
> +                        removeButton.disable();
> +                    }
> +                },
> +                specialkey: function (field, event) {
> +                    var removeButton = this.lookupReference('removeButton');
> +                    if (!removeButton.isDisabled() && event.getKey() == event.ENTER) {
> +                        removeButton.fireEvent('click', removeButton, event);
> +                    }
> +                }
> +            },
> +            'button[reference=removeButton]': {
> +                click: function () {
> +                    var view = this.getView();
> +                    Proxmox.Utils.API2Request({
> +                        url: view.getUrl() + view.getParams(),
> +                        method: 'DELETE',
> +                        waitMsgTarget: view,
> +                        failure: function (response, opts) {
> +                            view.close();
> +                            Ext.Msg.alert('Error', response.htmlStatus);
> +                        },
> +                        success: function (response, options) {
> +                            var hasProgressBar = view.showProgress &&
> +                            response.result.data ? true : false;
> +
> +                            if (hasProgressBar) {
> +                                // stay around so we can trigger our close events
> +                                // when background action is completed
> +                                view.hide();
> +
> +                                var upid = response.result.data;
> +                                var win = Ext.create('Proxmox.window.TaskProgress', {
> +                                    upid: upid,
> +                                    listeners: {
> +                                        destroy: function () {
> +                                            view.close();
> +                                        }
> +                                    }
> +                                });
> +                                win.show();
> +                            } else {
> +                                view.close();
> +                            }
> +                        }
> +                    });
> +                }
> +            }
> +        }
> +    },
> +
> +    items: [
> +        {
> +            xtype: 'component',
> +            cls: [Ext.baseCSSPrefix + 'message-box-icon',
> +                Ext.baseCSSPrefix + 'message-box-warning',
> +                Ext.baseCSSPrefix + 'dlg-icon']
> +        },
> +        {
> +            xtype: 'container',
> +            flex: 1,
> +            layout: {
> +                type: 'vbox',
> +                align: 'stretch'
> +            },
> +            items: [
> +                {
> +                    xtype: 'component',
> +                    reference: 'messageCmp'
> +                },
> +                {
> +                    itemId: 'confirmField',
> +                    reference: 'confirmField',
> +                    xtype: 'textfield',
> +                    name: 'confirm',
> +                    labelWidth: 300,
> +                    hideTrigger: true,
> +                    allowBlank: false
> +                },
> +                {
> +                    xtype: 'proxmoxcheckbox',
> +                    name: 'wipe',
> +                    reference: 'wipeCheckbox',
> +                    boxLabel: gettext('Wipe'),
> +                    checked: false,
> +                    autoEl: {
> +                        tag: 'div',
> +                        'data-qtip': gettext('Wipe disk after the removal of mountpoint')
> +                    }
> +                },
> +                {
> +                    xtype: 'container',
> +                    flex: 1,
> +                    layout: {
> +                        type: 'vbox',
> +                        align: 'middle'
> +                    },
> +                    height: 25,
> +                    items: [
> +                        {
> +                            xtype: 'component',
> +                            reference: 'noteCmp'
> +                        },
> +                    ]
> +                },
> +            ]
> +        }
> +    ],
> +    buttons: [
> +        {
> +            reference: 'removeButton',
> +            text: gettext('Remove'),
> +            disabled: true
> +        }
> +    ],
> +
> +    initComponent: function () {
> +        var me = this;
> +
> +        me.callParent();
> +
> +        var item = me.getItem();
> +
> +        if (!Ext.isDefined(item.id)) {
> +            throw "no ID specified";
> +        }
> +
> +        if (!Ext.isDefined(item.type)) {
> +            throw "no Disk type specified";
> +        }
> +
> +        var messageCmp = me.lookupReference('messageCmp');
> +        var noteCmp = me.lookupReference('noteCmp');
> +        var msg;
> +
> +        if (item.type === 'Dir') {
> +            msg = `Directory ${item.id} - Remove`
> +        } else {
> +            throw "unknown item type specified";
> +        }
> +
> +        if(me.getNote()) {
> +            noteCmp.setHtml(`<small>${me.getNote()}</small>`);
> +        }
> +
> +        messageCmp.setHtml(msg);
> +
> +        if (item.type === 'Dir') {
> +            let wipeCheckbox = me.lookupReference('wipeCheckbox');
> +            wipeCheckbox.setDisabled(true);
> +            wipeCheckbox.setHidden(true);
> +        }
> +
> +        var confirmField = me.lookupReference('confirmField');
> +        msg = gettext('Please enter the ID to confirm') +
> +            ' (' + item.id + ')';
> +        confirmField.setFieldLabel(msg);
> +    }
> +});
> 





^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-08-14  8:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-13 10:58 [pbs-devel] [PATCH proxmox-backup v3 0/2] removal of mount-units/directories Hannes Laimer
2020-08-13 10:58 ` [pbs-devel] [PATCH proxmox-backup v3 1/2] api2/node/../disks/directory: added DELETE endpoint for removal of mount-units Hannes Laimer
2020-08-14  5:38   ` [pbs-devel] applied: " Dietmar Maurer
2020-08-13 10:58 ` [pbs-devel] [PATCH proxmox-backup v3 2/2] ui/cli: added support for the " Hannes Laimer
2020-08-14  5:41   ` Dietmar Maurer
2020-08-14  8:13   ` Dominik Csapak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal