On Fri, Sep 30, 2016 at 12:23:17PM -0700, Robin H. Johnson wrote:
> Implement the QEMU Guest Agent sockets, so that code/scripts on the
> hypervisors can communicate with guest operating systems easily.
Awesome. This mostly looks good to me. See comments inline.
Cheers,
Brian.
> + # Add guest agent socket
> + if hvp[constants.HV_USE_GUEST_AGENT]:
> + qga_addr = utils.GetFreeSlot(pci_reservations, reserve=True)
pci_reservations isn't defined. Did you mean to derive this from
hvp[constants.HV_KVM_PCI_RESERVATIONS]? (NB make pylint would have caught this.)
> + qga_pci_info = ",bus=pci.0,addr=%s" % hex(qga_addr)
Nit: You could use "%#x" % addr instead of "%s" % hex(addr)
Nit: Also, I think this this would read a little better inlined into the device
string below.
> + qga_path = self._InstanceQemuGuestAgentMonitor(instance.name)
> + logging.info("KVM: Guest Agent available at %s", path)
Shouldn't this be qga_path? (Again, make pylint would have caught this)
> + # The 'qga0' identified can change, but the 'org.qemu.guest_agent.0'
> string is
Typo: s/identified/identifier/
> + # the default expected by the Guest Agent.
> + kvm_cmd.extend([
> + "-chardev",
> + "socket,path=%s,server,nowait,id=qga0" % qga_path,
> + ])
> + kvm_cmd.extend([
> + "-device",
> + "virtio-serial,id=qga0,%s" % qga_pci_info,
> + ])
> + kvm_cmd.extend([
> + "-device",
> + "virtserialport,chardev=qga0,name=org.qemu.guest_agent.0",
> + ])
Nit: Could you merge these 3 extends into 1 and maybe put the --opt and arg on
the same line?
