[libvirt] [PATCH v2 4/8] undefine: Implement internal API for qemu driver

[Eric Blake]

On 07/15/2011 03:06 AM, Osier Yang wrote:
> * src/qemu/qemu_driver.c: New call back for qemu_driver,
> New function qemudDomainUndefineFlags, and changes on
> qemudDomainUndefine.
> ---
>  src/qemu/qemu_driver.c |   35 ++++++++++++++++++++++++++++++++++-
>  1 files changed, 34 insertions(+), 1 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 3d4207e..0bf135d 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4303,12 +4303,18 @@ cleanup:
>      return dom;
>  }
>  
> -static int qemudDomainUndefine(virDomainPtr dom) {
> +static int
> +qemudDomainUndefineFlags(virDomainPtr dom,
> +                         unsigned int flags)

As long as we're touching this, s/qemudDomain/qemuDomain/

> +    name = qemuDomainManagedSavePath(driver, vm);
> +    if (name == NULL)
> +        goto cleanup;
> +
> +    if (virFileExists(name)) {
> +        if ((flags & VIR_DOMAIN_UNDEFINE_MANAGED_STATE)
> +            && (unlink(name) < 0)) {
> +            qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                            _("Failed on removing domain managed "

Grammar.

> +                            "state file '%s'"), name);

No need to expose our internal file name details to the user.

> +            goto cleanup;
> +        } else {
> +            qemuReportError(VIR_ERR_INTERNAL_ERROR,

Not an internal error.

> +                            _("Refusing to undefine with managed state "
> +                            "file '%s' exists"), name);

Grammar.

> +            goto cleanup;
> +        }

Logic bug.  This fails with "Refusing to undefine" if the unlink()
succeeds.  Rather, you want:

if (virFileExists(name)) {
    if (flags & VIR_DOMAIN_UNDEFINE_MANAGED_STATE) {
        if (unlink(name) < 0) {
            virReportSystemError(errno, "%s",
               _("Failed to remove managed state for domain"));
            goto cleanup;
        }
    } else {
        qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
           _("Refusing to undefine domain with managed state"));
        goto cleanup;
    }
}

Needs a v3.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org