On Tue, Feb 13, 2007 at 07:08:19PM +0000, Daniel P. Berrange wrote:
The attached patch implements the daemon for managing QEMU instances. The main changes from previous versions are:
- We can now talk to the QEMU monitor command line interface. This allows us to implement pause/resume/save/restore. Only the first two of those are done so far, and it needs more work to be truely robust.
- We now grok /proc/meminfo, /proc/cpuinfo and /proc/{pid}/stat to pull out node information, and guest runtime state. This needs a little more work for correct info on NUMA boxes and hyperthreading. The basic operation is correct though.
- All TCP / TLS stuff is removed, in favour of using the generic libvirtd for remote access.
just a few remarks to the code, feel free to commit to CVS, thanks ! [...]
+libvirt_qemud_CFLAGS = -D_XOPEN_SOURCE=600 -D_XOPEN_SOURCE_EXTENDED=1 -D_POSIX_C_SOURCE=199506L \
Hum, we really need all those flags ? Maybe one day libvirt should compile on some weird platform ;-)
+static int qemudParseUUID(const char *uuid, + unsigned char *rawuuid) {
need to be factored out somewhere but I'm sure we already agreed on that
+ if (macaddr) { + sscanf((const char *)macaddr, "%02x:%02x:%02x:%02x:%02x:%02x", + (unsigned int*)&net->mac[0], + (unsigned int*)&net->mac[1], + (unsigned int*)&net->mac[2], + (unsigned int*)&net->mac[3], + (unsigned int*)&net->mac[4], + (unsigned int*)&net->mac[5]); + } + + xmlFree(macaddr);
let's move it into if (macaddr) { block
+ return net; + + /* + error: + if (macaddr) + xmlFree(macaddr); + free(net); + return NULL; + */
if not needed anymore remove, unless it was needed for something more advanced
+ + +/* + * Parses a libvirt XML definition of a guest, and populates the + * the qemud_vm struct with matching data about the guests config + */ +static int qemudParseXML(struct qemud_server *server, + xmlDocPtr xml, + struct qemud_vm *vm) {
lot of that code could be cleaned up if I provided nicer high level function to do XPath extraction of values.
+/* + * Constructs a argv suitable for launching qemu with config defined + * for a given virtual machine. + */ +int qemudBuildCommandLine(struct qemud_server *server, + struct qemud_vm *vm, + char ***argv, + int *argc) { [...]
+ (*argv)[++n] = NULL;
how many args do we usually end up with :-) ?
+ return 0; + +/* Save a guest's config data into a persistent file */ +static int qemudSaveConfig(struct qemud_server *server, + struct qemud_vm *vm) { + char *xml; + int fd = -1, ret = -1; + int towrite; + struct stat sb; + + if (!(xml = qemudGenerateXML(server, vm))) { + return -1; + } + + if (stat(server->configDir, &sb) < 0) { + if (errno == ENOENT) { + if (mkdir(server->configDir, 0700) < 0) { + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, + "cannot create config directory %s", + server->configDir); + return -1; + } + } else { + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, + "cannot stat config directory %s", + server->configDir); + return -1; + }
Hum, should we check for ownership of that directory before writing in it, I'm not sure ...
+ } else if (!S_ISDIR(sb.st_mode)) { + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, + "config directory %s is not a directory", + server->configDir); + return -1; + } + + if ((fd = open(vm->configFile, + O_WRONLY | O_CREAT | O_TRUNC, + S_IRUSR | S_IWUSR )) < 0) { + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, + "cannot create config file %s", + vm->configFile); + goto cleanup; [...]
+/* Simple grow-on-demand string buffer */ +/* XXX re-factor to shared library */
+1 [...]
+ if (vm->def.id >= 0) { + if (qemudBufferPrintf(&buf, "<domain type='%s' id='%d'>\n", type, vm->def.id) < 0) + goto no_memory; + } else { + if (qemudBufferPrintf(&buf, "<domain type='%s'>\n", type) < 0) + goto no_memory; + }
Hum, did you change the format ? I remember you sent a mail about it and I'm afraid I forgot to answer at the time.
Index: qemud/config.h [...] +clientFunc funcsTransmitRW[QEMUD_PKT_MAX] = { + NULL, /* FAILURE code */ + qemudDispatchGetVersion, + qemudDispatchGetNodeInfo, + qemudDispatchListDomains, + qemudDispatchNumDomains, + qemudDispatchDomainCreate, + qemudDispatchDomainLookupByID, + qemudDispatchDomainLookupByUUID, + qemudDispatchDomainLookupByName, + qemudDispatchDomainSuspend, + qemudDispatchDomainResume, + qemudDispatchDomainDestroy, + qemudDispatchDomainGetInfo, + qemudDispatchDomainSave, + qemudDispatchDomainRestore, + qemudDispatchDumpXML, + qemudDispatchListDefinedDomains, + qemudDispatchNumDefinedDomains, + qemudDispatchDomainStart, + qemudDispatchDomainDefine, + qemudDispatchDomainUndefine +}; + +clientFunc funcsTransmitRO[QEMUD_PKT_MAX] = { + NULL, /* FAILURE code */ + qemudDispatchGetVersion, + qemudDispatchGetNodeInfo, + qemudDispatchListDomains, + qemudDispatchNumDomains, + NULL, + qemudDispatchDomainLookupByID, + qemudDispatchDomainLookupByUUID, + qemudDispatchDomainLookupByName, + NULL, + NULL, + NULL, + qemudDispatchDomainGetInfo, + NULL, + NULL, + qemudDispatchDumpXML, + qemudDispatchListDefinedDomains, + qemudDispatchNumDefinedDomains, + NULL, + NULL, + NULL, +};
I wonder if there would be any way to automatically exercise all that network glue code. Somthing like plugging the test driver on the server side for make check ... [...]
Index: qemud/qemud.c + for (i = 0; i < open_max; i++) + if (i != STDIN_FILENO && + i != STDOUT_FILENO && + i != STDERR_FILENO) + close(i);
I usually try to keep expressions fully parenthetized to avoid mistake Overall I'm surpized by how minimalist the changes are from normal QEmu to KVM, code reuse is nice :-) 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/