On Wed, Jul 20, 2011 at 02:33:42PM -0400, Ben Lipton wrote:
> p2v_transfer.py is run from the transfer OS to connect to the instance
> and perform the transfer. It takes three arguments:
> * Device that stores the root filesystem of the source OS
> * Name or address of instance to connect to
> * Path of a private key matching the public key installed on the
> instance.
>
> For the time being, the administrator needs to generate the keys and
> provide the private key to the user manually.
LGTM and will push, with some comments. You can either address them or
add TODOs/README notes about them:
> +def _WaitForCompletion(channel):
> + """Wait for a remote command to complete.
> +
> + Helper function that sleeps until the last command run by the channel has
> + completed.
> +
> + @type channel: paramiko.Channel
> + @param channel: The channel to wait for. If the command was run with
> + stdin, stdout, stderr = exec_command(), use stdout.channel.
> +
> + """
> + while not channel.exit_status_ready():
> + time.sleep(.01)
Hrmm, no way to break out of this function. Is is possible that the
channel gets confused and/or exit_status_ready won't become true for a
long time without any progress report to the user?
> +def PartitionTargetDisks(client, swap_cyls):
> + """Partition and format the disks on the target machine.
> +
> + Sends commands over the SSH connection to partition and format the
> + disk of the target instance.
> +
> + @type client: paramiko.SSHClient
> + @param client: SSH client object used to connect to the instance.
> + @type swap_cyls: int
> + @param swap_cyls: Desired size of swap space, in cylinders
> +
> + """
> + # Find out how many cylinders are available on target
> + total_cyls = 0
> + stdin, stdout, stderr = client.exec_command("sfdisk -l /dev/xvda")
Here and in the code below, you hardcode 'xvda'. This is correct only
for Xen, so I would recommend:
- abstract the device root (xvda) into a parameter to all functions
- read the available devices from the remote target at run-time
- ask the user if there's more than one block device available,
otherwise choose the first one
> +def UnmountSourceFilesystems():
> + """Undo mounts performed by MountSourceFilesystems.
> +
> + Unmounts all filesystems mounted by MountSourceFilesystems. Currently,
> since
> + only the root filesystem ever gets mounted, this only unmounts the root
> + filesystem.
> +
> + """
> + try:
> + subprocess.check_call(["umount", SOURCE_MOUNT])
subprocess.check_call was added in 2.5; currently, all ganeti code is
2.4 compatible, so a README mention would be welcome.
thanks,
iustin
