Re: [libvirt] [PATCH 2/5] virsh: Fix semantics of --config for "update-device" command

On 03/27/13 16:22, Laine Stump wrote: > On 03/21/2013 12:42 PM, 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 >> true if the machine is running. > > Are you certain of that? My understanding was that when --config was > specified, *only* the persistent config should be changed, but not the > live state, regardless of whether or not the domain is running. > Otherwise, there is no way to change just the persistent config of a > running domain. Yes, that's why I'm doing this. Before this patch --config meant "modify the persistent config along with the live change" and there was no way to use this virsh command to change the next-boot config if the domain was running. > > Or is your explanation incorrect, and the code correct? > > Here is what I *thought* was the meaning of these options: > > --config - only change the persistent config, but not the > live state. The command should fail for a transient > domain. > > --live - only change the live state, but not the persistent config. > The command should fail for a domain that isn't running. > > --current - useless (really, I mean that) because its meaning is > different > depending on whether or not the domain is running Yes those we understand in the same way. > > --persistent - deprecated synonym for --config This used to be the synonym for config, but this might be undestood as the option you are missing later on ... > > no option - also useless because it means the same thing as --current > > What's missing: a way to say "change both live state and persistent > config as appropriate" Well, this patch would abuse the --persistent flag for this purpose. It adds _CONFIG always and _LIVE in case the domain is up. > > After discussing it on the list, in the name of consistency I actually > very reluctantly implemented this same logic for virsh net-update even > though I thought it was terrible. Did I get it wrong? No, you've got it correct. Well, except for --persistent but I'm less sure about that than you. > > So what is the *real* meaning of each of these options? (and are you > sure you're not changing the meaning of any of them?) Except for --persistent, the meaning seems to be well defined now and used appropriately in new places. The problem is with the legacy API's that didn't take up on this approach yet.