On 10/29/12 10:38, Jiri Denemark wrote:
On Thu, Oct 25, 2012 at 16:37:37 +0200, Ján Tomko wrote:
In the XML warning, this prints uuids for domain names with special
characters in them and shell-escaped names for other elements (like
snapshosts and networks) with special names.
---
When saving snapshots, the domain name is appended to the
"snapshot-edit" command, so using a domain name that needs escaping
would lead to something that can't be just fed to the shell as it would
glue them together.
diff to xml: shell-escape domain name in comment:
- Domain names don't get escaped, UUIDs are preferred.
- The command gets escaped too.
diff to v1: don't try to use CDATA (it doesn't belong there)
---
src/conf/domain_conf.c | 6 +++-
src/libvirt_private.syms | 2 +
src/qemu/qemu_domain.c | 3 +-
src/util/buf.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++
src/util/buf.h | 1 +
src/util/xml.c | 62 ++++++++++++++++++++++---------------------
src/util/xml.h | 1 +
7 files changed, 109 insertions(+), 32 deletions(-)
I think we're making this too complicated for no real benefit. The goal is to
provide a hint to anyone who looks at the autogenerated XML files and IMHO
providing an escaped string (that would only work in environments for which it
was designed). I would just keep it simple:
- emit "virsh command name" in case name is nice
- emit "virsh command uuid" in case name is ugly and uuid is known
- emit "virsh command" in all other cases
This looks reasonable.
This should keep the hints in domain and network XML files in /etc/libvirt
usable for copy&paste (the UUID fallback works there). Snapshot files (located
in /var/lib/libvirt) use "virsh snapshot-edit domain snapshot", where domains
are passed as part of the command and snapshots have no UUIDs. Thus to keep the
code simple, I'd emit just "virsh snapshot-edit", which is still a useful
hint and I don't believe we need to do anything beyond that.
Well, we might check if the names contain dashes, and then just print
the line without the names. But that's probably not worth even trying.
Putting the static hint should be good enough. I'd just put there tokens
that the user should replace for real values:
"virsh snapshot-edit 'domain-name' 'snapshot-name'" or something like that.
A 109+/32- patch doesn't look worth of the usability improvement that is
marginal.
Peter
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list