[libvirt] [PATCH] Add warning message to XML definition files stored on disk
Users often edit XML domain file stored in configuration directory thinking of modifying a domain/network/pool/etc. Thus it is wise to let them know they are using the wrong way and give them hint. --- src/conf/domain_conf.c | 2 ++ src/conf/network_conf.c | 1 + src/conf/nwfilter_conf.c | 3 +++ src/conf/storage_conf.c | 2 ++ src/libvirt_private.syms | 1 + src/util/util.c | 22 ++++++++++++++++++++++ src/util/util.h | 2 ++ 7 files changed, 33 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2a681d9..4cd1228 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8543,6 +8543,7 @@ int virDomainSaveConfig(const char *configDir, VIR_DOMAIN_XML_WRITE_FLAGS))) goto cleanup; + virSavePrependWarning(&xml); if (virDomainSaveXML(configDir, def, xml)) goto cleanup; @@ -8563,6 +8564,7 @@ int virDomainSaveStatus(virCapsPtr caps, if (!(xml = virDomainObjFormat(caps, obj, flags))) goto cleanup; + virSavePrependWarning(&xml); if (virDomainSaveXML(statusDir, obj->def, xml)) goto cleanup; diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 5738757..82f10e6 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -959,6 +959,7 @@ int virNetworkSaveConfig(const char *configDir, if (!(xml = virNetworkDefFormat(def))) goto cleanup; + virSavePrependWarning(&xml); if (virNetworkSaveXML(configDir, def, xml)) goto cleanup; diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 09dc32b..fcd0b24 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2282,6 +2282,7 @@ int virNWFilterSaveConfig(const char *configDir, if (!(xml = virNWFilterDefFormat(def))) goto cleanup; + virSavePrependWarning(&xml); if (virNWFilterSaveXML(configDir, def, xml)) goto cleanup; @@ -2635,6 +2636,8 @@ virNWFilterObjSaveDef(virNWFilterDriverStatePtr driver, return -1; } + virSavePrependWarning(&xml); + if ((fd = open(nwfilter->configFile, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR )) < 0) { diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 116898d..96aa567 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1542,6 +1542,8 @@ virStoragePoolObjSaveDef(virStorageDriverStatePtr driver, return -1; } + virSavePrependWarning(&xml); + if ((fd = open(pool->configFile, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR )) < 0) { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1b22be6..f2c005e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -946,6 +946,7 @@ virRandom; virRandomInitialize; virRun; virRunWithHook; +virSavePrependWarning; virSetBlocking; virSetCloseExec; virSetInherit; diff --git a/src/util/util.c b/src/util/util.c index 1bb0328..c6a4634 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -3204,3 +3204,25 @@ bool virIsDevMapperDevice(const char *devname ATTRIBUTE_UNUSED) return false; } #endif + +int virSavePrependWarning(char **xml) { + char *retPtr = NULL; + int ret; + + ret = virAsprintf(&retPtr, "%s\n%s", + _("<!--\n\ +WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE\n\ +OVERWRITTEN AND LOST. Changes to xml configurations should be made using virsh\n\ +edit or other application using the libvirt API. See:\n\ +\n\ +http://wiki.libvirt.org/wiki/index.php?title=FAQ#Where_are_VM_config_files_stored.3F_How_do_I_edit_a_VM.27s_XML_config.3F\n\ +-->\n"), + *xml); + + if (ret < 0) + return ret; + + VIR_FREE(*xml); + *xml = retPtr; + return ret; +} diff --git a/src/util/util.h b/src/util/util.h index d320c40..a6a91de 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -299,4 +299,6 @@ int virBuildPathInternal(char **path, ...) ATTRIBUTE_SENTINEL; char *virTimestamp(void); bool virIsDevMapperDevice(const char *devname) ATTRIBUTE_NONNULL(1); + +int virSavePrependWarning(char **xml); #endif /* __VIR_UTIL_H__ */ -- 1.7.4.4
On 04/28/2011 12:34 PM, Michal Privoznik wrote:
Users often edit XML domain file stored in configuration directory thinking of modifying a domain/network/pool/etc. Thus it is wise to let them know they are using the wrong way and give them hint. --- src/conf/domain_conf.c | 2 ++ src/conf/network_conf.c | 1 + src/conf/nwfilter_conf.c | 3 +++ src/conf/storage_conf.c | 2 ++ src/libvirt_private.syms | 1 + src/util/util.c | 22 ++++++++++++++++++++++ src/util/util.h | 2 ++ 7 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2a681d9..4cd1228 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8543,6 +8543,7 @@ int virDomainSaveConfig(const char *configDir, VIR_DOMAIN_XML_WRITE_FLAGS))) goto cleanup;
+ virSavePrependWarning(&xml); if (virDomainSaveXML(configDir, def, xml)) goto cleanup;
@@ -8563,6 +8564,7 @@ int virDomainSaveStatus(virCapsPtr caps, if (!(xml = virDomainObjFormat(caps, obj, flags))) goto cleanup;
+ virSavePrependWarning(&xml); if (virDomainSaveXML(statusDir, obj->def, xml)) goto cleanup;
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 5738757..82f10e6 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -959,6 +959,7 @@ int virNetworkSaveConfig(const char *configDir, if (!(xml = virNetworkDefFormat(def))) goto cleanup;
+ virSavePrependWarning(&xml); if (virNetworkSaveXML(configDir, def, xml)) goto cleanup;
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 09dc32b..fcd0b24 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2282,6 +2282,7 @@ int virNWFilterSaveConfig(const char *configDir, if (!(xml = virNWFilterDefFormat(def))) goto cleanup;
+ virSavePrependWarning(&xml); if (virNWFilterSaveXML(configDir, def, xml)) goto cleanup;
@@ -2635,6 +2636,8 @@ virNWFilterObjSaveDef(virNWFilterDriverStatePtr driver, return -1; }
+ virSavePrependWarning(&xml); + if ((fd = open(nwfilter->configFile, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR ))< 0) { diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 116898d..96aa567 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1542,6 +1542,8 @@ virStoragePoolObjSaveDef(virStorageDriverStatePtr driver, return -1; }
+ virSavePrependWarning(&xml); + if ((fd = open(pool->configFile, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR ))< 0) { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1b22be6..f2c005e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -946,6 +946,7 @@ virRandom; virRandomInitialize; virRun; virRunWithHook; +virSavePrependWarning; virSetBlocking; virSetCloseExec; virSetInherit; diff --git a/src/util/util.c b/src/util/util.c index 1bb0328..c6a4634 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -3204,3 +3204,25 @@ bool virIsDevMapperDevice(const char *devname ATTRIBUTE_UNUSED) return false; } #endif + +int virSavePrependWarning(char **xml) { + char *retPtr = NULL; + int ret; + + ret = virAsprintf(&retPtr, "%s\n%s", + _("<!--\n\ +WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE\n\ +OVERWRITTEN AND LOST. Changes to xml configurations should be made using virsh\n\ +edit or other application using the libvirt API. See:\n\
either s/or other/or another/ or else s/application/applications/
+\n\ +http://wiki.libvirt.org/wiki/index.php?title=FAQ#Where_are_VM_config_files_stored.3F_How_do_I_edit_a_VM.27s_XML_config.3F\n\ +-->\n"),
Can this URL change if the wiki is edited? Do we maybe want to put this info in a less modifiable place? (Dunno, just asking.)
+ *xml); + + if (ret< 0) + return ret; + + VIR_FREE(*xml);
I'm not really comfortable that it requires the caller to initialize to 0. But that may be just me - anyone else have an opinion?
+ *xml = retPtr; + return ret; +} diff --git a/src/util/util.h b/src/util/util.h index d320c40..a6a91de 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -299,4 +299,6 @@ int virBuildPathInternal(char **path, ...) ATTRIBUTE_SENTINEL; char *virTimestamp(void);
bool virIsDevMapperDevice(const char *devname) ATTRIBUTE_NONNULL(1); + +int virSavePrependWarning(char **xml); #endif /* __VIR_UTIL_H__ */
On Thu, Apr 28, 2011 at 01:42:37PM -0400, Laine Stump wrote:
On 04/28/2011 12:34 PM, Michal Privoznik wrote:
Users often edit XML domain file stored in configuration directory thinking of modifying a domain/network/pool/etc. Thus it is wise to let them know they are using the wrong way and give them hint. --- src/conf/domain_conf.c | 2 ++ src/conf/network_conf.c | 1 + src/conf/nwfilter_conf.c | 3 +++ src/conf/storage_conf.c | 2 ++ src/libvirt_private.syms | 1 + src/util/util.c | 22 ++++++++++++++++++++++ src/util/util.h | 2 ++ 7 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2a681d9..4cd1228 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8543,6 +8543,7 @@ int virDomainSaveConfig(const char *configDir, VIR_DOMAIN_XML_WRITE_FLAGS))) goto cleanup;
+ virSavePrependWarning(&xml); if (virDomainSaveXML(configDir, def, xml)) goto cleanup;
@@ -8563,6 +8564,7 @@ int virDomainSaveStatus(virCapsPtr caps, if (!(xml = virDomainObjFormat(caps, obj, flags))) goto cleanup;
+ virSavePrependWarning(&xml); if (virDomainSaveXML(statusDir, obj->def, xml)) goto cleanup;
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 5738757..82f10e6 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -959,6 +959,7 @@ int virNetworkSaveConfig(const char *configDir, if (!(xml = virNetworkDefFormat(def))) goto cleanup;
+ virSavePrependWarning(&xml); if (virNetworkSaveXML(configDir, def, xml)) goto cleanup;
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 09dc32b..fcd0b24 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2282,6 +2282,7 @@ int virNWFilterSaveConfig(const char *configDir, if (!(xml = virNWFilterDefFormat(def))) goto cleanup;
+ virSavePrependWarning(&xml); if (virNWFilterSaveXML(configDir, def, xml)) goto cleanup;
@@ -2635,6 +2636,8 @@ virNWFilterObjSaveDef(virNWFilterDriverStatePtr driver, return -1; }
+ virSavePrependWarning(&xml); + if ((fd = open(nwfilter->configFile, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR ))< 0) { diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 116898d..96aa567 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1542,6 +1542,8 @@ virStoragePoolObjSaveDef(virStorageDriverStatePtr driver, return -1; }
+ virSavePrependWarning(&xml); + if ((fd = open(pool->configFile, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR ))< 0) { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1b22be6..f2c005e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -946,6 +946,7 @@ virRandom; virRandomInitialize; virRun; virRunWithHook; +virSavePrependWarning; virSetBlocking; virSetCloseExec; virSetInherit; diff --git a/src/util/util.c b/src/util/util.c index 1bb0328..c6a4634 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -3204,3 +3204,25 @@ bool virIsDevMapperDevice(const char *devname ATTRIBUTE_UNUSED) return false; } #endif + +int virSavePrependWarning(char **xml) { + char *retPtr = NULL; + int ret; + + ret = virAsprintf(&retPtr, "%s\n%s", + _("<!--\n\ +WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE\n\ +OVERWRITTEN AND LOST. Changes to xml configurations should be made using virsh\n\ +edit or other application using the libvirt API. See:\n\
either s/or other/or another/ or else s/application/applications/
I think it's ok as written, but another would also be correct.
+\n\ +http://wiki.libvirt.org/wiki/index.php?title=FAQ#Where_are_VM_config_files_stored.3F_How_do_I_edit_a_VM.27s_XML_config.3F\n\ +-->\n"),
Can this URL change if the wiki is edited? Do we maybe want to put this info in a less modifiable place? (Dunno, just asking.)
That's a good point. If we're going to point people at a URL, it should be a static page. (My bad for suggesting that URL in the first place.)
+ *xml); + + if (ret< 0) + return ret; + + VIR_FREE(*xml);
I'm not really comfortable that it requires the caller to initialize to 0. But that may be just me - anyone else have an opinion?
+ *xml = retPtr; + return ret; +} diff --git a/src/util/util.h b/src/util/util.h index d320c40..a6a91de 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -299,4 +299,6 @@ int virBuildPathInternal(char **path, ...) ATTRIBUTE_SENTINEL; char *virTimestamp(void);
bool virIsDevMapperDevice(const char *devname) ATTRIBUTE_NONNULL(1); + +int virSavePrependWarning(char **xml); #endif /* __VIR_UTIL_H__ */
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/28/2011 10:34 AM, Michal Privoznik wrote:
Users often edit XML domain file stored in configuration directory thinking of modifying a domain/network/pool/etc. Thus it is wise to let them know they are using the wrong way and give them hint.
+++ b/src/conf/domain_conf.c @@ -8543,6 +8543,7 @@ int virDomainSaveConfig(const char *configDir, VIR_DOMAIN_XML_WRITE_FLAGS))) goto cleanup;
+ virSavePrependWarning(&xml); if (virDomainSaveXML(configDir, def, xml))
Prepending can be expensive - in the middle of the operation, we have to have twice the space for xml malloc'd, and we are also spending time on a memcpy from old memory to new. Is there any way to instead change virDomainSaveXML and friends to take a flags argument, and pass 0 from most callers but VIR_XML_EMIT_WARNING from these particular callers; then if the flag is set, they call the common helper function: virEmitXMLWarning(int fd) which outputs the message directly into the target file prior to outputting xml? That would be less memory pressure and copying involved. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (4)
-
Dave Allan -
Eric Blake -
Laine Stump -
Michal Privoznik