[libvirt] [PATCH 8/8] UndefineFlags: Extend virsh undefine to support new flag

[Osier Yang]

于 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.
>