Re: [Libguestfs] [PATCH v3 11/13] v2v: add --in-place mode
Can you repost the final 3 patches, rebased on top of current master?
More comments below ...
On Tue, Oct 20, 2015 at 04:08:19PM +0300, Roman Kagan wrote:
+ let guestfs_kind = (match conversion_mode with
+ | Copying (_, _) -> "overlay"
+ | In_place -> "source VM"
+ ) in
+
+ message (f_"Opening the %s") guestfs_kind;
This is a bit cleaner if written as:
(match conversion_mode with
| Copying _ -> message (f_"Opening the overlay")
| In_place -> message (f_"Opening the source VM")
);
It's obviously shorter, but there's another reason for doing this
which is it allows the translators to see the complete message for
better translation (although in this case it doesn't make a huge
difference).
You'll have to do the same change with the "Closing ..." message,
since guestfs_kind is no longer defined.
- let keep_serial_console = output#keep_serial_console in
+ let keep_serial_console = (match conversion_mode with
+ | Copying (_, _) -> output#keep_serial_console
+ | In_place -> true
+ ) in
I don't think this change is correct. I think you should call
#keep_serial_console in both cases, since this is controlled by the
guest type, not the conversion mode.
- message (f_"Closing the overlay");
+ message (f_"Closing the %s") guestfs_kind;
See above.
+and populate_disks (g:G.guestfs) src_disks =
As far as I know, the type annotation here should not be necessary.
But if it is necessary, then put some spaces in, ie:
... (g : G.guestfs) ...
+ List.iter (
+ fun ({s_qemu_uri = qemu_uri; s_format = format}) ->
+ match format with
+ | None ->
+ g#add_drive_opts qemu_uri ~cachemode:"unsafe"
~discard:"besteffort"
+ | Some fmt ->
+ g#add_drive_opts qemu_uri ~format:fmt ~cachemode:"unsafe"
+ ~discard:"besteffort"
You don't need the match statement here, just use ?format
as a parameter, ie:
fun ({s_qemu_uri = qemu_uri; s_format = format}) ->
g#add_drive_opts qemu_uri ?format
~cachemode:"unsafe" ~discard:"besteffort"
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/