On Tue, Feb 13, 2007 at 07:03:42PM +0000, Daniel P. Berrange wrote:
The attached patch implements the library driver for QEMU.
The driver is pretty much identical in style to the xen proxy driver. There are two supported URLs:
qemu:///session - a per-user (private) daemon.Can be run by unprivileged users. Config files kept in $HOME/.qemu and the socket is in the abstract namespace at $HOME/.qemu/sock The daemon for session instance is spawned on demand
qemu:///system - a per-machine (public) daemon. Must be launched ahead of time by root. Config files kept in /etc/qemu and socket on the FS at /var/run/qemu/sock & sock-ro The read-write socket is restricted to root only, while the read-only socket is public.
This patch also:
- makes virsh use a read-write connection by default - adds extra error info to virterror & friends
Looks just fine, but I still have a few comments see below :-) [...]
+ /* 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 !
+ +int qemuListDomains(virConnectPtr conn, + int *ids, + int maxids){ + struct qemud_packet req, reply; + int i, nDomains; + + req.header.type = QEMUD_PKT_LIST_DOMAINS; + req.header.dataSize = 0; + + if (qemuProcessRequest(conn, NULL, &req, &reply) < 0) { + return -1; + } + + 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.
+ for (i = 0 ; i < nDomains ; i++) { + ids[i] = reply.data.listDomainsReply.domains[i]; + } + + return nDomains; +} + + +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
+ return NULL; + } +
+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 :-).
+#ifndef __VIR_QEMU_INTERNAL_H__ +#define __VIR_QEMU_INTERNAL_H__ + +#include <libvirt/virterror.h> + +#ifdef __cplusplus +extern "C" { +#endif + + void qemuRegister(void); + +#ifdef __cplusplus +} +#endif +#endif /* __VIR_QEMU_INTERNAL_H__ */
Feel free to push to CVS, thanks ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/