On Tue, Feb 13, 2007 at 04:06:17PM -0500, Daniel Veillard wrote:
On Tue, Feb 13, 2007 at 07:03:42PM +0000, Daniel P. Berrange wrote:
+ /* Block sending entire outgoing packet */ + while (outLeft) { + int got = write(conn->handle, out+outDone, outLeft);
stylistic, I prefer to have the variables defined at the function level, but I could understand why one would argue otherwise :-) Appears in many other places, definitely not a blocker though !
Personally I'd define them at point of first use, but we're not using C99 in libvirt, so I'll settle for start of nearest nested code block
+ nDomains = reply.data.listDomainsReply.numDomains; + if (nDomains > maxids) + return -1;
Is the semantic really to error instead of truncating in that case ? it seems to me the code in other drivers just pass the first maxids ones.
Yeah, I'll just clamp it to maxids.
+virDomainPtr +qemuDomainCreateLinux(virConnectPtr conn, const char *xmlDesc, + unsigned int flags ATTRIBUTE_UNUSED){ + struct qemud_packet req, reply; + virDomainPtr dom; + int len = strlen(xmlDesc); + + if (len > (QEMUD_MAX_XML_LEN-1)) {
maybe we need to provide a clear error there
Definitely. There's a bunch of other places which need more error reporting in both driver & daemon. I've fixed up all the critical path ones which you will encounter commonly, but still more low priority ones to deal with.
+int qemuListDefinedDomains(virConnectPtr conn, +int qemuDomainCreate(virDomainPtr dom) { +virDomainPtr qemuDomainDefineXML(virConnectPtr conn, const char *xml) { +int qemuUndefine(virDomainPtr dom) {
Seems to me all of these drivers entry point should be made static since they are not exported from the .h, right ? Only the registration routine ought to be exported (very clean :-).
Sounds reasonable. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|