4:04 p.m.
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/