Re: [pbs-devel] [PATCH vma-to-pbs v5 1/4] add support for bulk import of a dump directory

[Filip Schauer <f.schauer@proxmox.com>]

On 13/11/2024 12:41, Shannon Sterz wrote:
>>   const CMD_HELP: &str = "\
>>   Usage: vma-to-pbs [OPTIONS] --repository <auth_id@host:port:datastore> --vmid <VMID> [vma_file]
>>
>>   Arguments:
>> -  [vma_file]
>> +  [vma_file | dump_directory]
>>
>>   Options:
>>         --repository <auth_id@host:port:datastore>
>>             Repository URL
>>         [--ns <NAMESPACE>]
>>             Namespace
>> -      --vmid <VMID>
>> +      [--vmid <VMID>]
> nit: this is marked as optional here (and in the code), but the usage
> line above still make it look like it's required.

That usage line describes the command for a single VMA file. In that
case `--vmid` actually is required. It is however not required for bulk
import. So to make that clearer I made two usage lines in v6.


On 13/11/2024 12:41, Shannon Sterz wrote:
>> +    let vma_file: Box<dyn BufRead> = match &backup_args.compression {
>> +        Some(compression) => {
>> +            let vma_file_path = backup_args
>> +                .vma_file_path
>> +                .as_ref()
>> +                .expect("No VMA file path provided");
>> +            let mut cmd = match compression {
>> +                Compression::Zstd => {
>> +                    let mut cmd = Command::new("zstd");
>> +                    cmd.args(["-q", "-d", "-c"]);
>> +                    cmd
> i think the following would be more elegant here:
>
>
> ```rs
>                     Compression::Zstd => Command::new("zstd")
>                         .args(["-q", "-d", "-c"]),
> ```
>
> it's a bit more concise imo
>
>> +                }
>> +                Compression::Lzo => {
>> +                    let mut cmd = Command::new("lzop");
>> +                    cmd.args(["-d", "-c"]);
>> +                    cmd
> same as above

Yeah I tried that, but unfortunatelly it does not compile:

```
error[E0716]: temporary value dropped while borrowed
    --> src/vma2pbs.rs:532:38
     |
531 |             let mut cmd = match compression {
     |                 ------- borrow later stored here
532 |                 Compression::Zstd => 
Command::new("zstd").args(["-q", "-d", "-c"]),
     | ^^^^^^^^^^^^^^^^^^^^                        - temporary value is 
freed at the end of this statement
     |                                      |
     |                                      creates a temporary value 
which is freed while still in use
     |
     = note: consider using a `let` binding to create a longer lived value
```



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel