fix new 1.81.0 warning and clippy error

[iximeow]

on 1.81, this expression caused a rustc warning, and clippy error:

warning: this function depends on never type fallback being `()`
   --> bin/propolis-cli/src/main.rs:497:1
    |
497 | / async fn migrate_instance(
498 | |     src_client: Client,
499 | |     dst_client: Client,
500 | |     src_addr: SocketAddr,
501 | |     dst_uuid: Uuid,
502 | |     disks: Vec<DiskRequest>,
503 | | ) -> anyhow::Result<()> {
    | |_______________________^
    |
    = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #123748 <https://github.com/rust-lang/rust/issues/123748>
    = help: specify the types explicitly
note: in edition 2024, the requirement `!: FromIterator<()>` will fail
   --> bin/propolis-cli/src/main.rs:598:20
    |
598 |         .collect::<anyhow::Result<_>>()?;
    |                    ^^^^^^^^^^^^^^^^^
    = note: `#[warn(dependency_on_unit_never_type_fallback)]` on by default

warning: `propolis-cli` (bin "propolis-cli") generated 1 warning

i am, honestly, entirely missing how the never type is involved here, or how changing the collect's type from anyhow::Result<_> to anyhow::Result<()> changes things in a direction to satisfy rustc. but bounding the type further doesn't seem like it would cause a problem?

it made my totally unrelated change in #759 fail :(

edit: there were a few new warnings that also get denied in CI and are fixed here, but the one in this message is still especially confusing.

[iximeow]

this suggestion kinda makes my skin crawl. "inspect_err" sounds a lot like you should expect println!() or something boring to follow, but on the next like we are destroying a resource because of the error. i'm not interested in adding #![allow(clippy::manual_inspect)] in an ad-hoc manner at crate roots though, so.. this what the suggested fix looks like.

[gjcolombo]

Optional, just noodling on the problem a bit: does it look any better to write it out by hand?

let vm = VmmFd::open(&name);
if vm.is_err() {
    let _ = ctl.vm_destroy(name.as_bytes());
}
let vm = vm?;

Or we could just write out the ? match explicitly. I don't have especially strong feelings here--it does seem like inspect is not quite the right verb for what we're trying to do here, but I'm not too sure map was either, since this wasn't transforming the error at all. Up to you in any case.

[iximeow]

i'd given that one a try and realized i was having a hard time reading it after the fact, since the ? and exit control flow is separated away from .is_err(). an explicit match would probably read fine though, i'll do that in a sec

[dancrossnyc]

sorry for the drive-by comment, but might this be or_else?

let vm = VmmFd::open(&name).or_else(|e| {
    let _ = ctl.vm_destroy(name.as_bytes());
    Err(e)
})?;

To me, the or_else says "we're doing something if this fails."

[iximeow]

i'd have liked or_else a lot but it turns out that is also on the path to inspect_err: that results in Clippy offering "using Result.or_else(|x| Err(y)), which is more succinctly expressed as map_err(|x| y). it's right, but then with that transform Clippy points towards inspect_err. 🙃

anyway, i've adjusted this to be an explicit match on both cases. seems fine.

[dancrossnyc]

Aww Clippy, why ya gotta be like that? :-/

[gjcolombo]

Optional, just noodling on the problem a bit: does it look any better to write it out by hand?

let vm = VmmFd::open(&name);
if vm.is_err() {
    let _ = ctl.vm_destroy(name.as_bytes());
}
let vm = vm?;

Or we could just write out the ? match explicitly. I don't have especially strong feelings here--it does seem like inspect is not quite the right verb for what we're trying to do here, but I'm not too sure map was either, since this wasn't transforming the error at all. Up to you in any case.