Hugh Brock wrote:
virDomainDefineXML should overwrite an existing inactive domain config if it finds one; it does this successfully for later (post 3.0.3) xen versions. However on xen 3.0.3 overwriting an existing config would fail with a name conflict. This patch allows the overwrite, removing the prior config from the config hashes.If someone more familiar than I with libvirt error handling and style could look at this and tell me if I've done something horribly wrong, I'd appreciate it.Signed-off-by: Hugh Brock <hbrock@xxxxxxxxxx>
Thinking aloud and trying to untangle the various hashes used ...
Index: src/xm_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xm_internal.c,v retrieving revision 1.23 diff -u -r1.23 xm_internal.c --- src/xm_internal.c 4 Apr 2007 14:19:49 -0000 1.23 +++ src/xm_internal.c 1 May 2007 17:23:05 -0000 @@ -2072,7 +2072,9 @@ */ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) { virDomainPtr ret; + virDomainPtr olddomain; char filename[PATH_MAX]; + const char * oldfilename; unsigned char uuid[VIR_UUID_BUFLEN]; virConfPtr conf = NULL; xenXMConfCachePtr entry = NULL; @@ -2103,8 +2105,44 @@ }if (virHashLookup(nameConfigMap, value->str)) {- xenXMError(conn, VIR_ERR_INTERNAL_ERROR, "domain with name already exists"); - goto error; + /* domain exists, we will overwrite it */ + + if (!(oldfilename = (char *)virHashLookup(nameConfigMap, value->str))) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, "can't retrieve config filename for domain to overwrite"); + goto error; + }
So get the filename from the nameConfigMap. Looks OK, although you could have also got the filename from configDir ++ "/" ++ value->str, but no matter.
+ if (!(entry = virHashLookup(configCache, oldfilename))) {
+ xenXMError(conn, VIR_ERR_INTERNAL_ERROR, "can't retrieve config entry for domain to overwrite");
+ goto error;
+ }
Get the xenXMConfCache object from the configCache keyed on filename. Looks OK.
Despite the name, configCache isn't literally a cache. Rather it's a list of filename -> config entries for everything under /etc/xen.
+ if (xenXMConfigGetUUID(entry->conf, "uuid", uuid) < 0) {
+ xenXMError(conn, VIR_ERR_INTERNAL_ERROR, "uuid config parameter is missing");
+ goto error;
+ }
Get the UUID from the config file.
+ if (!(olddomain = virGetDomain(conn, value->str, uuid))) + goto error;
Get the old domain, given name & UUID.
+ if (olddomain->id != -1) {
+ xenXMError(conn, VIR_ERR_INTERNAL_ERROR, "can't overwrite an active domain");
+ goto error;
+ }
+
+ /* Remove the name -> filename mapping */
+ if (virHashRemoveEntry(nameConfigMap, value->str, NULL) < 0) {
+ xenXMError(conn, VIR_ERR_INTERNAL_ERROR, "failed to remove old domain from config map");
+ goto error;
+ }
Remove the old mapping in nameConfigMap.
+ /* Remove the config record itself */
+ if (virHashRemoveEntry(configCache, oldfilename, xenXMConfigFree) < 0) {
+ xenXMError(conn, VIR_ERR_INTERNAL_ERROR, "failed to remove old domain from config map");
+ goto error;
+ }
Remove the configCache entry.
+ entry = NULL;
}
if ((strlen(configDir) + 1 + strlen(value->str) + 1) > PATH_MAX) {
and then drop through to virConfWriteFile which will blindly overwrite the file.
Looks A-OK to me! Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 Directors: Michael Cunningham (USA), Charlie Peters (USA) and David Owens (Ireland)
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature