Code review comment for lp:~soren/nova/libvirt-is-more-than-kvm-and-qemu

Revision history for this message
Rick Harris (rconradharris) wrote :

Nice work.

A couple of tiny nits:

> 30 + if len(arch_nodes):

Usually better to just write:

  if arch_nodes:

> 56 + while topology_node != None:

Comparisons to None should use identity not equivalence:

  while topology_node is not None:

or if its None-ness doesn't matter:

  while topology_node:

> 8 - if FLAGS.connection_type != 'libvirt':
> 9 + if (FLAGS.connection_type != 'libvirt' or
> 10 + (FLAGS.connection_type == 'libvirt' and
> 11 + FLAGS.libvirt_type not in ['kvm', 'qemu'])):
> 12 msg = _('Only KVM is supported for now. Sorry!')
> 13 raise exception.Error(msg)

The logic differs from the logging. Is live-migration supported for both KVM and QEMU now? If so, we should probably update the logging to reflect that: "Only KVM or QEMU is supported.."

review: Needs Fixing

« Back to merge proposal