On Wed, Nov 15, 2006 at 02:33:26AM +0000, Daniel P. Berrange wrote:
> This is the main patch. It adds a new 'xm config file' backend driver to
> libvirt. This basically just scans /etc/xen for config files, reads them
> and turns them into libvirt XML to create domains. Similarly it can accept
> libvirt XML and create config files from it.
Haha, the whole point of the exercise :-)
> Some important points
>
> - It can override /etc/xen with LIBVIRT_XM_CONFIG_DIR env variable
> - It only re-scans /etc/xen every X seconds & caches parsed config
> files in-memory to avoid parsing / I/O overhead
> - Automatically generates a UUID if the config file doesn;t have one
> - Skips over a blacklist of files from /etc/xen which are known to
> not be valid config files to avoid too many error messages
> - Filters list of inactive domains to strip out any which are currently
> running (acccording to XenD)
> - Allows changing of VCPUS, memory & max memory config settings and
> writes changes back out to disk
> - Can start, create, delete domains from disk
> + /* This method is called by various methods to scan /etc/xen
> + (or whatever directory was set by LIBVIRT_XM_CONFIG_DIR
> + environment variable) and process any domain configs. It
> + has rate-limited so never rescans more frequently than
> + once every X seconds */
Right, I don't want to be chastanized publicly by Dave Jones again
at OLS 2007 :-)
> + static int xenXMConfigCacheRefresh(void) {
[...]
> + /* Skip anything which isn't a file (takes care of scripts/ subdir */
> + if ((stat(path, &st) < 0) ||
> + !S_ISREG(st.st_mode)) {
I would put extra braces (!S_ISREG(st.st_mode)) ... paranoid as usual :-)
> + memset(info, 0, sizeof(virDomainInfo));
> + if (xenXMConfigGetInt(entry->conf, "memory", &mem) < 0 ||
> + mem < 0)
> + info->memory = 64 * 1024;
64 MB is a minimal memory limit that we are referencing in various places
in libvirt for Xen back-ends, maybe that ought to be isolated in internal.h
> + virBufferAdd(buf, "<domain type='xen'>\n", -1);
> + virBufferVSprintf(buf, " <name>%s</name>\n", name);
Okay one of the things which scares me a bit here is encoding for the
strings. We generate XML files without any encoding information which means
it has to be UTF-8 (or UTF-16 but clearly it's not), it was more or less
fine to do those direct buffers write before because we were in a contained
environment, but now those strings come from external files, and well
a chinese person may genuinely want to use chinese name in his config file
and use BIG5 encoding for example, and in such case the config file generated
here would not be XML. I assume config files generated by virt-manager
when creating new domains are likely to use the strings returned from
Gnome widget, I wonder if it is UTF-8 or the encoding of the user's locale.
I guess the simplest and cleanest at this point would be to make sure
anything parsed by conf.[ch] is actually UTF-8, and maybe double check
virt-manager string generation.
> + virBufferVSprintf(buf, " <graphics type='vnc' port='%d'/>\n", (5900+display));
Shouldn't the 5900 magic number be in a header too ?
> + /*
> + * Start a domain from an existing defined config file
> + */
> + int xenXMDomainCreate(virDomainPtr domain) {
> + char *xml;
> + char *sexpr;
> + int ret, xendConfigVersion;
> + unsigned char uuid[16];
> +
> + if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
> + xenXMError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
> + __FUNCTION__);
> + return(-1);
> + }
> +
> + if (domain->handle != -1)
> + return (-1);
> + if (domain->conn->flags & VIR_CONNECT_RO)
> + return (-1);
> +
> + if (!(xml = xenXMDomainDumpXML(domain, 0)))
> + return (-1);
> +
> + if ((xendConfigVersion = xend_get_config_version(domain->conn)) < 0) {
> + xenXMError(domain->conn, VIR_ERR_INTERNAL_ERROR, "cannot determine xend config version");
> + return (-1);
> + }
> + printf("'%s'\n", xml);
> + if (!(sexpr = virDomainParseXMLDesc(xml, NULL, xendConfigVersion))) {
> + free(xml);
> + return (-1);
> + }
> + free(xml);
> + printf("'%s'\n", sexpr);
> + ret = xenDaemonDomainCreateLinux(domain->conn, sexpr);
> + free(sexpr);
> + if (ret != 0) {
> + fprintf(stderr, "Failed to create domain %s\n", domain->name);
> + return (-1);
> + }
> +
> + ret = xend_wait_for_devices(domain->conn, domain->name);
> + if (ret != 0) {
> + fprintf(stderr, "Failed to get devices for domain %s\n", domain->name);
> + return (-1);
> + }
> +
> + if ((ret = xenDaemonDomainLookupByName_ids(domain->conn, domain->name, uuid)) < 0) {
> + return (-1);
> + }
> + domain->handle = ret;
> +
> + ret = xenDaemonDomainResume(domain);
> + if (ret != 0) {
> + fprintf(stderr, "Failed to resume new domain %s\n", domain->name);
> + xenDaemonDomainDestroy(domain);
> + domain->handle = -1;
> + return (-1);
> + }
> +
> + return 0;
> + }
Good to finally have the bridge from config files to libvirt loading :-)
Very cool ! Push it :-)
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/