于 2011年06月22日 05:37, Eric Blake 写道:
On 06/21/2011 03:07 AM, Osier Yang wrote:
This patch add two new options (--live, --current),
Good.
and changes
"--persistent" into "--config", just as other similar commands,
NACK. Consistency may be nice, but backwards compatibility is more
important. If we were to create an aliasing mechanism, where
--persistent is still recognized as an alias for --config but not
explicitly documented in the help output (but still mentioned in
virsh.pod), then we could use that; in fact, we could add:
VSH_OFLAG_ALIAS - if set, then the help field is the primary spelling
for which this option spelling is an alias
{"config", VSH_OT_BOOL, 0, N_("affect next boot")},
{"persistent", VSH_OT_BOOL, VSH_OT_ALIAS, "config"},
where only --config is documented in 'virsh help attach-device', but
where 'virsh attach-device --persistent' behaves the same. But that's a
much bigger prerequisite patch; the rest of your patch (adding --live
and --current) is good whether or not we figure out a way to
backwards-compatibly rename --persistent into --config.
Sounds good idea, :-), will try this.
This patch removes codes like this, leave the determination for underly
s/underly/underlying/
OK.
hypervisor driver.
---
tools/virsh.c | 242
++++++++++++++++++++++++++++++++++++++++++---------------
1 files changed, 178 insertions(+), 64 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index abc4614..4cd2e1a 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -9738,7 +9738,9 @@ static const vshCmdInfo info_attach_device[] = {
static const vshCmdOptDef opts_attach_device[] = {
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or
uuid")},
{"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("XML file")},
- {"persistent", VSH_OT_BOOL, 0, N_("persist device attachment")},
+ {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
+ {"live", VSH_OT_BOOL, 0, N_("affect running domain")},
+ {"current", VSH_OT_BOOL, 0, N_("affect current domain")},
Maybe it's a bit early morning for me, but I didn't get this
description. We are always affecting current domain :)
How about a separate cleanup patch which unifies all of these similar
strings into:
"config/persistent",... N_("alter persistent configuration, effect
observed on next boot, error for transient domain")
"live",... N_("alter running domain, immediate effect, error for
transient domain")
"current",... N_("either --config or --live according to current domain
state")
This can be both of "--live" and "--config" for an active domain,
depends on how underlying hypervisor driver implements it.
or is that too long?
Regards
Osier
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list