Libguestfs - May 2020 - [PATCH v2v] v2v: Remove extraneous '=' when setting --bandwidth/--bandwidth-file.

Trivial fix.

We really need a regression test for all v2v inpus related to nbdkit.
There is actually nothing at all at the moment.  Of course if it was
easy to test inputs over the network from a ‘make check’ rule then
we'd be doing it already.

I thought about adding something like a ‘-it file’ option which would
use nbdkit-file-plugin for test only.  However I'm rather cautious
about adding new virt-v2v command line features, especially dubious
do-nothing features, when my longer term plan is to break apart
virt-v2v into more easily consumable pieces.  That will only make the
long term plan more difficult.

We could probably test ‘-it ssh’ using the same trick that nbdkit uses
(https://github.com/libguestfs/nbdkit/blob/master/tests/web-server.h#L33)
but even that is not straightforward as ‘-it ssh’ also wants to run
scp commands.

So that's a long way of saying this fixes the code but doesn't add a
regression test.

Rich.

Commit c3a54d6aed6dfc65f9ffa59976bb8d20044c03a8 ("v2v: Add standalone
nbdkit module.") was supposed to be a simple refactoring but it broke
the --bandwidth and --bandwidth-file options (amongst other things).

Because of an extra '=' character which was accidentally left over, it
would add an extra character in the nbdkit-rate-filter command line.
For example:

  virt-v2v .. --bandwidth 200M

would invoke:

  nbdkit .. --filter rate rate==200M

which causes a parse error.  The --bandwidth-file option does not
invoke a parse error but does not work, for similar reasons.

Thanks: Ming Xie
---
 v2v/nbdkit_sources.ml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/v2v/nbdkit_sources.ml b/v2v/nbdkit_sources.ml
index 979c37734d..402dfd0e55 100644
--- a/v2v/nbdkit_sources.ml
+++ b/v2v/nbdkit_sources.ml
@@ -118,11 +118,11 @@ let common_create ?bandwidth ?extra_debug ?extra_env
plugin_name plugin_args           let args             match bandwidth with
            | StaticBandwidth rate ->
-              [ "rate=", rate ]
+              [ "rate", rate ]
            | DynamicBandwidth (None, filename) ->
-              [ "rate-file=", filename ]
+              [ "rate-file", filename ]
            | DynamicBandwidth (Some rate, filename) ->
-              [ "rate=", rate; "rate-file=", filename ] in
+              [ "rate", rate; "rate-file", filename ] in
          cmd, args
     )
     else cmd, [] in
-- 
2.26.2

On Thursday, 28 May 2020 12:51:16 CEST Richard W.M. Jones
wrote:
> Commit c3a54d6aed6dfc65f9ffa59976bb8d20044c03a8 ("v2v: Add standalone
> nbdkit module.") was supposed to be a simple refactoring but it broke
> the --bandwidth and --bandwidth-file options (amongst other things).
> 
> Because of an extra '=' character which was accidentally left over,
it
> would add an extra character in the nbdkit-rate-filter command line.
> For example:
> 
>   virt-v2v .. --bandwidth 200M
> 
> would invoke:
> 
>   nbdkit .. --filter rate rate==200M
> 
> which causes a parse error.  The --bandwidth-file option does not
> invoke a parse error but does not work, for similar reasons.
> 
> Thanks: Ming Xie
> ---
Remember to mention RHBZ#1841096 in the commit message.

Otherwise, LGTM.

-- 
Pino Toscano