Hi David,
Looks pretty cool, some comments in now particular order:
- At first I thought the pygrub thing isn't needed - but you're right,
for PV it's not enough to say "boot this image from its MBR", you
need to say "the assumptions that pygrub makes about there being a
grub config file etc. will hold true"
- Including vcpu, memory, graphics and nic in this metadata is mixing
up two things - the things the image need in order to boot and the
defaults recommended when instantiating a guest with the image.
Perhaps put them in a different toplevel element e.g.
<image>
<boot_options>
<boot>
...
</boot_options>
<storage>
<disk>
...
</storage>
<defaults>
<memory>
<vcpus>
...
</defaults>
</image>
- The disk -> target mapping thing is pretty strange - I'd suggest
that the order the disks are listed in just specifies the target.
I think I see your problem, though, you've got 2 boot disks and you
need to make sure the HVM disk is first under HVM. How about if you
could just specify the filename of the disk image to boot from in
<loader> ... the logic to get the libvirt XML right is a bit hairy,
but should be doable. Is it hd/cdrom, make sure the boot disk is
the first disk etc.
<loader disk="boot-hvm.img" />
- You don't have a human readable name for a UI that allows people to
choose from a number of images.
- From what I can see, you still have the ability to create a disk at
instantiation time, but not format it?
- If you added ImageInstaller support to virt-install, couldn't
virt-image almost just run virt-install rather than re-implementing
a lot of it? i.e. at a glance, it looks like you've forked
virt-install in order to have a version with a simpler command
line. I don't have a problem with virt-image, and the simpler
command line, it's the copied and pasted code I don't like.
- This looks odd:
+ order = [ "xen", "kvm", "kqemu", "qemu" ]
+ for o in order:
+ if types.count(o) > 0:
i.e. it'd be better not to have to keep that list updated. How
about you just pick the first type? If libvirt doesn't order the
list of available types in a useful order, maybe it should?
- You don't have a way of specifying the disk image should be
read-only.
- Shouldn't we be copying even system disk images, unless they're
read-only, on instantiation - i.e. you should be able to run
virt-image multiple times.
Cheers,
Mark.
