Re: VM pools and libgovirt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Christophe,

On Tue, Dec 3, 2013 at 5:12 AM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
Thanks for the patch, a few quick initial comments:
- formatting of the commit log was off, see (for example)
  https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_GIT_commit_message_structure
  for the recommended format of git commit logs (helps when using 'git shortlog' for example)
  This document as a whole is an interesting read imo ;)

Thanks for the tip!
 
- I'd use ovirt_vmpool_allocate_vm (rather than _allocatevm)

I used allocatevm because the action is actually "allocatevm" not "allocate_vm", but if you think the function name doesn't need to be in line with the action, then I agree.
 
- imo the ovirt_invoke_action stuff should be moved to a generic place
  before this patch, but I can look into that

Yes, it's basically duplicated code, but I didn't think it was up to me to restructure :).
 
- there were a few minor white space issues (see patch below)

My editor was misconfigured...
 
- you need to export the new public functions in govirt/govirt.sym (see
  patch below)

Yes, I meant to ask you about that, but I wasn't sure what version to put, so I left it up to you (which was a good idea I think :))


Would you like me to generate a patch with all the suggested changes and a better log entry?

Cheers!
iordan

--
The conscious mind has only one thread of execution.
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]