[Ovirt-devel] [PATCH] Add vnc port to database.

[Ian Main]

On Mon, 16 Jun 2008 09:16:23 +0200
Chris Lalancette <clalance at redhat.com> wrote:

> Ian Main wrote:
> > This patch parses the xml from the newly created vm to get the vnc port
> > and adds it to the database.  If the host is stopped the port is set
> > to nil.  This works for stopping of vms including the case where the
> > node reboots because taskomatic is notified through host-status.
> > 
> > There is also some blanks removed and an indentation fixup.
> 
> Looks good, overall.  One quick comment below:
> 
> > @@ -319,6 +322,13 @@ def start_vm(task)
> >      conn = Libvirt::open("qemu+tcp://" + host.hostname + "/system")
> >      dom = conn.define_domain_xml(xml.to_s)
> >      dom.create
> > +
> > +    doc = REXML::Document.new(dom.xml_desc)
> > +    attrib = REXML::XPath.match(doc, "//graphics/@port")
> > +    if not attrib.empty?:
> > +      vm.vnc_port = attrib.to_s.to_i
> > +    end
> 
> Good, I see you've also duplicated this in restore_vm now.  So, next step, how
> about a method :).

Yeah I was thinking of that.. Problem is it's not exactly duplicated.. only 2 lines are the same, well 3 if you count the empty check.  I could probably have it return an int or nil on empty though, which would make it reasonably refactored.

  Ian