public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>,
	Dominik Csapak <d.csapak@proxmox.com>,
	Dylan Whyte <d.whyte@proxmox.com>
Subject: Re: [pbs-devel] [PATCH V2 proxmox-backup] Fix 3335: Allow removing datastore contents on delete
Date: Mon, 21 Feb 2022 14:39:03 +0100	[thread overview]
Message-ID: <05266c31-3897-aed9-abe8-69ec07e84e44@proxmox.com> (raw)
In-Reply-To: <4dec537e-4526-0db7-bb06-bf25a3c04ddb@proxmox.com>

On 21.02.22 12:10, Dominik Csapak wrote:
> while i know that we don't recommend having other data in the datastore,
> imho it's not wise to remove *everything* in the datastore path
> 
> AFAICT we can know what we touch (ct/host/vm/.chunk dirs, .gc-status, .lock)
> so we could only remove that?
> 

meh, I'm torn a bit; While PBS owns the datastore directory and this
is really fine to do IMO, it'd be relatively simple to avoid doing, as
like you say, there are few and always known top-level directories we need
to iterate over; if empty I'd also delete the top-level directory itself
though.

> also we should probably remove the datastore from the config before
> removing files from it? otherwise i can start deleting
> and another user could start his backup which will
> interfere with each other and leave the datastore in an
> undefined state?

it should be put in maintenance state prior to starting off deletion,
that handles this already nicely and can set a readable info message; once
it's finally applied that is.

> 
> what we at least should have here is some mechanism that nobody
> can backup/restore/etc. from a datastore while it's deleted

see above

>> +    let info = &api2::config::datastore::API_METHOD_DELETE_DATASTORE;
>> +    let result = match info.handler {
>> +        ApiHandler::Async(handler) => (handler)(param, info, rpcenv).await?,
>> +        _ => unreachable!(),
>> +    };
>> +
>> +    crate::wait_for_local_worker(result.as_str().unwrap()).await?;
>> +    Ok(Value::Null)
>> +
> 
> i know the other functions in this file do it exactly like this,
> but i'd probably prefer to have a http client here to
> do the deletion via the api? (like we do in most other cases)

hmm, while I agree in general for most commands I'm not so sure about this
specific one; this is something a admin may need to do when no space is left
anymore, having less parts involved could make it more robust in some cases.




  reply	other threads:[~2022-02-21 13:39 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-26 15:38 Dylan Whyte
2022-02-07 14:39 ` Oguz Bektas
2022-02-21 11:10 ` Dominik Csapak
2022-02-21 13:39   ` Thomas Lamprecht [this message]
2022-02-21 13:49     ` Dominik Csapak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=05266c31-3897-aed9-abe8-69ec07e84e44@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=d.whyte@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal