On Friday, 3 February 2017 15:27:42 CET Richard W.M. Jones wrote:
On Thu, Feb 02, 2017 at 02:34:24PM +0100, Pino Toscano wrote:
Parse the output disk as URI, and use all its attributes just like it is done for the input disk. The only change is that the fsync of the output disk is limited now for local URIs only, since it will not work with remote protocols. --- resize/resize.ml | 43 +++++++++++++++++++++++++++++++------------ resize/virt-resize.pod | 6 +++--- 2 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/resize/resize.ml b/resize/resize.ml index 19cd8df..7e16cb5 100644 --- a/resize/resize.ml +++ b/resize/resize.ml @@ -315,6 +315,13 @@ read the man page virt-resize(1). error (f_"error parsing URI '%s'. Look for error messages printed above.") infile in
+ (* outfile can be a URI. *) + let outfile = + try (outfile, URI.parse_uri outfile) + with Invalid_argument "URI.parse_uri" -> + error (f_"error parsing URI '%s'. Look for error messages printed above.") + outfile in
This matches what we do with 'infile', but I wonder if that isn't wrong, ie. we should warn or even ignore if we couldn't parse the infile / outfile as a URI and construct a local file URI.
I am not sure about that -- IMHO it is much better to tell the user right away that the URI specified is not valid, and thus virt-resize (and the other tools too) cannot do anything with them. My gut feeling is that trying to use the invalid URI as local file might do more harm than good -- after all, local files (absolute and relative paths) are supported just fine already.
The following would be a bit safer if all the new variables it introduces were scoped, ie:
let () =
+ let _, { URI.path = path; protocol = protocol; + server = server; username = username; + password = password } = outfile in (* The output disk is being created, so use cache=unsafe here. *) g#add_drive ?format:output_format ~readonly:false ~cachemode:"unsafe" - outfile; + ~protocol ?server ?username ?secret:password path; if not (quiet ()) then Progress.set_up_progress_bar ~machine_readable g; g#launch () ^ in
Indeed -- I was trying to use currying here, but with labelled arguments it is kind of messy and overly complicated (for not much as real benefit). Thanks, -- Pino Toscano