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 -=|