Re: [libvirt] [PATCH RFC] virsh: Fix semantics of --config for "update-device" command
On 03/15/2013 10:37 AM, Peter Krempa wrote:
The man page states that with --config the next boot is affected.
This
can be understood as if _only_ the next bood was affected. This isn't
s/bood/boot/
true if the machine is running.
This patch adds the full --live, --config, --current infrastructure and
tweaks stuff to correctly support the obsolete --persistent flag.
---
Notes:
- This patch will be greatly simplified with macros from:
http://www.redhat.com/archives/libvir-list/2013-March/msg00268.html
- There are multiple places like this in virsh that will need update too.
(detach-device for example)
I take it you plan on doing those after seeing how this RFC goes?
- https://bugzilla.redhat.com/show_bug.cgi?id=921398
tools/virsh-domain.c | 53 +++++++++++++++++++++++++++++++++++++++++-----------
tools/virsh.pod | 22 +++++++++++++++-------
2 files changed, 57 insertions(+), 18 deletions(-)
+++ b/tools/virsh-domain.c
@@ -9304,13 +9304,21 @@ static const vshCmdOptDef opts_update_device[] = {
.help = N_("XML file")
},
{.name = "persistent",
- .type = VSH_OT_ALIAS,
- .help = "config"
+ .type = VSH_OT_BOOL,
+ .help = N_("make live change persistent")
So you are documenting this option again, now that it doesn't match up
to any of the other options.
+++ b/tools/virsh.pod
+If I<--live> is specified, affect a running domain.
+If I<--config> is specified, affect the next startup of a persistent domain.
+If I<--current> is specified, affect the current domain state.
+Both I<--live> and I<--config> flags may be given, but I<--current>
is
+exclusive. Not specifying any flag is the same as specifying I<--current>.
+
+For compatibility purposes, I<--persistent> is alias of I<--config> and
+I<--live> if the domain is running. This flag isn't compatible with the
+other domain status flags.
Rather:
For compatibility purposes, I<--persistent> behaves like I<--config> for
an offline domain, and like I<--live> I<--config> for a running domain.
Why must we force mutual exclusion? That is, --persistent --current
doesn't make sense, but --persistent --config is just redundant, and
--persistent --live is redundant for a running domain (it would fail for
an offline domain, though). In fact, the semantics of --persistent seem
a bit nicer; should we be adding this option to all the commands that
support both --config and --live, since it is friendlier to type a
single option that behaves correctly according to domain state than it
is to figure out whether it is safe to use --live?
At any rate, I think this patch is moving in the right direction.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)