Re: [Libvir] PATCH 1/2 QEMU driver - internal driver
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 -=|