public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Niko Fellner <n.fellner@logics.de>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] parallelize restore.rs fn restore_image: problems in async move
Date: Sat, 5 Dec 2020 01:11:52 +0000	[thread overview]
Message-ID: <AM0PR09MB2754A1792BFCBA4610F335C2F2F00@AM0PR09MB2754.eurprd09.prod.outlook.com> (raw)
In-Reply-To: <cf954288-3a94-d460-9e10-82a06d6b8c9d@proxmox.com>

> first thing: the error you get is simply a return type mismatch, the 'async move' block expects to return a Result<(), Error> because there is a bail!() and multiple '?' calls.
> the simplest way to fix this is to write
> Ok(())
> at the end of the async move block
> the next problem you will run into is that there are multiple uses of variables that get moved in a loop (which does not work in rust) so you have to pull out some things, and clone some others

@Dominik: Thanks a lot for your help! Yes, and after following your instructions I've discovered more and more similarities with "pull.rs"... 

> a much bigger problem is the following:
> the callbacks that get called (write_data_callback/write_zero_callback)
> are not 'Send' meaning they cannot send between threads safely (needed by tokio::spawn) because they both are closures that use a '*mut T'
> in that code this would not actually be a problem, since the restored blocks never overlap, but rust does not know that
> without digging further, i think this is not easily solvable in rust without either writing unsafe code or changing our c <-> rust api (idk if that is possible)

So that's not only a restriction of tokio::spawn, but also anything that Rayon provides (including its ParallelBridge?), and the code in pull.rs, too? (ParallelHandler, futures::stream, ...) ?

> as pointed out in the bug report, we already have such code in pull.rs
> Why not use that instead of something new?

@Dietmar: You're right, that looks like a gold mine - but unfortunately one that I cannot mine due to my limited knowledge of Rust...  Maybe you can help out to apply its magic on restore.rs? Most importantly, will it work with that write_data_callback and write_zero_callback which are not Send?

> this is not easily solvable in rust without either writing unsafe code or changing our c <-> rust api (idk if that is possible)

"Unsafe" seems to be the right keyword here. In proxmox-backup-qemu the CallbackPointers (capi_types.rs) do this: "unsafe impl std::marker::Send". Those CallbackPointers are used by lots of "_async" functions within the c <-> rust api. So I guess a hypothetic proxmox_restore_image_async needs to use CallbackPointers, too?



  reply	other threads:[~2020-12-05  1:12 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-04 14:14 Niko Fellner
2020-12-04 14:59 ` Dominik Csapak
2020-12-05  1:11   ` Niko Fellner [this message]
2020-12-04 15:05 ` Dietmar Maurer
2020-12-05 23:39 Niko Fellner

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=AM0PR09MB2754A1792BFCBA4610F335C2F2F00@AM0PR09MB2754.eurprd09.prod.outlook.com \
    --to=n.fellner@logics.de \
    --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