On 03.02.2012 20:34, Laine Stump wrote:
> This allows virsh to use the new VIR_DOMAIN_DESTROY_GRACEUL flag for
> virDomainDestroyFlags.
> ---
> Okay, how about this one :-)
>
> tools/virsh.c | 9 ++++++++-
> tools/virsh.pod | 6 +++++-
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 42985a9..6a151e6 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -4264,6 +4264,7 @@ static const vshCmdInfo info_destroy[] = {
>
> static const vshCmdOptDef opts_destroy[] = {
> {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
> + {"graceful", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("needs rawio capability")},
> {NULL, 0, 0, NULL}
> };
>
> @@ -4273,6 +4274,7 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd)
> virDomainPtr dom;
> bool ret = true;
> const char *name;
> + int result;
I tend to define unsigned flags = 0; here, even when they'll be used
only for one flag right now, but it will simplify patches adding a new one.
>
> if (!vshConnectionUsability(ctl, ctl->conn))
> return false;
> @@ -4280,7 +4282,12 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd)
> if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
> return false;
>
> - if (virDomainDestroy(dom) == 0) {
> + if (vshCommandOptBool(cmd, "graceful"))
> + result = virDomainDestroyFlags(dom, VIR_DOMAIN_DESTROY_GRACEFUL);
> + else
> + result = virDomainDestroy(dom);
> +
> + if (result == 0) {
So this chunk will look like:
if (vshCommandOptBool(cmd, "graceful"))
flags |= VIR_DOMAIN_DESTROY_GRACEFUL;
if (flags)
result = virDomainDestroyFlags(dom, flags);
else
result = virDomainDestroy(dom);
if (result == 0) {
But this is not a show stopper for me.
> vshPrint(ctl, _("Domain %s destroyed\n"), name);
> } else {
> vshError(ctl, _("Failed to destroy domain %s"), name);
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 10b51ff..a85da13 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -458,7 +458,7 @@ Flag I<--title> selects operation on the title field instead of description.
> If neither of I<--edit> and I<--new_desc> are specified the note or description
> is displayed instead of being modified.
>
> -=item B<destroy> I<domain-id>
> +=item B<destroy> I<domain-id> [I<--graceful>]
>
> Immediately terminate the domain domain-id. This doesn't give the domain
> OS any chance to react, and it's the equivalent of ripping the power
> @@ -472,6 +472,10 @@ be lost once the guest stops running, but the snapshot contents still
> exist, and a new domain with the same name and UUID can restore the
> snapshot metadata with B<snapshot-create>.
>
> +If I<--graceful> is specified, don't resort to extreme measures
> +(e.g. SIGKILL) when the guest doesn't stop after a reasonable timeout;
> +return an error instead.
> +
> =item B<domblkstat> I<domain> I<block-device> [I<--human>]
>
> Get device block stats for a running domain. A I<block-device> corresponds
Nice, you haven't forgot the documentation.
Overall, ACK, regardless you'll change that nit or not.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list