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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list