all lists on 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 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