[libvirt] [PATCH 8/8] UndefineFlags: Extend virsh undefine to support new flag
Osier Yang
jyang at redhat.com
Thu Jul 14 03:56:37 UTC 2011
于 2011年07月14日 02:23, Eric Blake 写道:
> On 07/13/2011 04:19 AM, Osier Yang wrote:
>> ---
>> tools/virsh.c | 12 +++++++++++-
>> tools/virsh.pod | 6 +++++-
>> 2 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index 3cdf043..f81e923 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -1409,6 +1409,7 @@ static const vshCmdInfo info_undefine[] = {
>>
>> static const vshCmdOptDef opts_undefine[] = {
>> {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name or uuid")},
>> + {"undefine-managed-state", VSH_OT_BOOL, 0, N_("remove domain managed state file")},
> Dave suggested '--remove-managed-state', but given the name of our
> existing virsh command:
>
> managedsave-remove
>
> I'd feel slightly better with:
>
> virsh undefine domain --managed-save
>
> [Meanwhile, we don't have any virsh command that directly exposes
> virDomainHasManagedSaveImage - that information should probably be added
> as a flag to an existing command, perhaps 'virsh list --managed-save'
> adding a column on whether a managed save image exists for each domain.]
This will be added in v2,
>> + int flags = 0;
>> + int undefine_managed_state = vshCommandOptBool(cmd, "undefine-managed-state");
>> +
>> + if (undefine_managed_state)
>> + flags |= VIR_DOMAIN_UNDEFINE_MANAGED_STATE;
>> +
>> + if (!undefine_managed_state)
>> + flags = -1;
>>
>> if (!vshConnectionUsability(ctl, ctl->conn))
>> return false;
>> @@ -1440,7 +1449,8 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
>> VSH_BYNAME|VSH_BYUUID)))
>> return false;
>>
>> - if (virDomainUndefine(dom) == 0) {
>> + if (((flags == -1) ? virDomainUndefine(dom) :
>> + virDomainUndefineWithFlags(dom, flags)) == 0) {
> Most times when we add a new flag to virsh, we can't support it without
> using the new API. But here, we can, and so I think we should. That
> is, if you virsh 0.9.4 as the client, and are talking to two different
> servers (one 0.9.3 and one 0.9.4), you get the following behavior for a
> domain that has a managed save image:
>
> server 0.9.3:
> virsh undefine dom -> call virDomainHasManagedSaveImage - if that is
> true, then refuse to run virDomainUndefine (thus papering over the 0.9.3
> safety bug). If false, then virDomainUndefine is safe.
> virsh undefine dom --managed-save -> tries virDomainUndefineFlags, which
> fails with unimplemented. Falls back to combo
> virDomainManagedSaveRemove/virDomainUndefine, which does the job we want.
>
> server 0.9.4
> virsh undefine dom -> call virDomainHasManagedSaveImage - if that is
> true, then refuse to run virDomainUndefine. If false, then
> virDomainUndefine is safe.
> virsh undefine dom --managed-save -> tries virDomainUndefineFlags, which
> succeeds and does the job we want (or fails, but with an error different
> than unimplemented)
>
> an alternative would be that calling 'virsh undefine dom' without flags
> skips virDomainHasManagedSaveImage, and just blindly calls
> virDomainUndefine - in that case, a server at 0.9.4 would properly fail,
> but a server at 0.9.3 would still expose the bug that we are trying to
> avoid of leaving stale managed state behind.
>
>
> For precedence in making virsh try a fallback to older API when the new
> API is unsupported, see the setvcpus command.
>
>> +++ b/tools/virsh.pod
>> @@ -804,11 +804,15 @@ hypervisor.
>> Output the device used for the TTY console of the domain. If the information
>> is not available the processes will provide an exit code of 1.
>>
>> -=item B<undefine> I<domain-id>
>> +=item B<undefine> I<domain-id> optional I<--undefine-managed-state>
>>
>> Undefine the configuration for an inactive domain. Since it's not running
>> the domain name or UUID must be used as the I<domain-id>.
> Oh, this is different than libvirt.c, which claimed you can run
> 'undefine' on a running persistent domain. Which is it? Can undefine
> make a running domain transient, or must it be on an inactive domain?
No, document in libvirt.c is not correct, undefining on a running domain
will be refused.
>>
>> +If I<--undefine-managed-state> is specified, the managed state file will
>> +be removed along with the domain undefine peocess, the entire domain
>> +undefine process will fail if it fails on removing the managed state file.
> Given my above thoughts (and once I validate the behavior of undefine on
> a running domain), I'd word this as:
>
> =item B<undefine> I<domain-id> [I<--managed-save>]
>
> Undefine the configuration for a domain. If domain is not running, the
> domain name or UUID must be used as the I<domain-id>; if the domain is
> running, then this converts a persistent domain to a transient domain.
>
> The I<--managed-save> flag guarantees that any managed state (see the
> B<managesave> command) is also cleaned up. Without the flag, attempts
> to undefine a domain with managed state will fail.
>
More information about the libvir-list
mailing list